From cb50ca1f15525fd4e7a1366dead1baaa4070380a Mon Sep 17 00:00:00 2001 From: bill-parker Date: Fri, 7 Aug 2026 11:37:04 +0100 Subject: [PATCH 01/15] refactor: extend make_request for multipart uploads and YAML responses Add files and response_format params to PlatformAPIHandler.make_request. When files is set, Content-Type is omitted so requests can set the multipart boundary automatically. YAML responses are parsed via ruamel.yaml. --- src/poly/handlers/platform_api.py | 43 +++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/src/poly/handlers/platform_api.py b/src/poly/handlers/platform_api.py index 64c86500..337f0397 100644 --- a/src/poly/handlers/platform_api.py +++ b/src/poly/handlers/platform_api.py @@ -10,6 +10,7 @@ from concurrent.futures import ThreadPoolExecutor, as_completed import requests +from ruamel.yaml import YAML from poly.constants import DEFAULT_VOICE_ID_FALLBACK, DEFAULT_VOICE_IDS from poly.utils import any_credentials_exist, retrieve_api_key @@ -91,19 +92,28 @@ def make_request( data: ty.Optional[dict] = None, params: ty.Optional[dict] = None, headers: ty.Optional[dict] = None, + files: ty.Optional[dict] = None, + response_format: str = "json", use_jupiter_api: bool = False, ) -> dict: """Make a request to the Platform API. Args: - region (str): The region name - endpoint (str): The API endpoint - method (str): The HTTP method - data (dict | None): The request body for POST/PUT requests - params (dict | None): Query string parameters + region (str): The region name. + endpoint (str): The API endpoint. + method (str): The HTTP method. + data (dict | None): The request body for POST/PUT requests. + params (dict | None): Query string parameters. + headers (dict | None): Override headers. Built automatically when None. + files (dict | None): Multipart file upload fields. When set, ``data`` + is ignored and ``Content-Type`` is omitted so ``requests`` can set + the multipart boundary automatically. + response_format (str): How to parse the response. "json" (default) + or "yaml". + use_jupiter_api (bool): Whether to use the Jupiter API. Returns: - dict: The response JSON + dict: The parsed response. """ url = PlatformAPIHandler.get_base_url(region, use_jupiter_api) + endpoint correlation_id = f"adk-{uuid.uuid4()}" @@ -112,9 +122,10 @@ def make_request( headers = { "X-API-KEY": retrieve_api_key(region), "X-PolyAI-Correlation-Id": correlation_id, - "Content-Type": "application/json", "X-Poly-Source": "adk", } + if not files: + headers["Content-Type"] = "application/json" logger.info(f"Making {method} request to {url}") @@ -125,7 +136,8 @@ def make_request( headers=headers, params=params, allow_redirects=False, - data=json.dumps(data) if data else None, + files=files, + data=json.dumps(data) if data and not files else None, ) logger.debug( @@ -137,7 +149,8 @@ def make_request( api_response.raise_for_status() except requests.HTTPError: logger.debug( - f"Error in request status_code={api_response.status_code!r} response={api_response.text!r}" + f"Error in request status_code={api_response.status_code!r}" + f" response={api_response.text!r}" ) raise @@ -145,13 +158,21 @@ def make_request( logger.info(f"Request to {url} successful (no content)") return {} + if response_format == "yaml": + content = api_response.text.strip() + if not content: + return {} + ry = YAML() + result = ry.load(content) + return dict(result) if result else {} + try: - api_response = api_response.json() + parsed = api_response.json() except json.JSONDecodeError as e: raise ValueError(f"Failed to parse JSON response: {e}") logger.info(f"Request to {url} successful") - return api_response + return parsed @staticmethod def get_accessible_regions(regions: list[str]) -> list[str]: From d6b8a83a1c085ba44689b9c041576116ce51cbe8 Mon Sep 17 00:00:00 2001 From: bill-parker Date: Fri, 7 Aug 2026 11:38:03 +0100 Subject: [PATCH 02/15] feat: add custom metrics platform API methods Add URL constants and 5 methods to PlatformAPIHandler for custom metrics: get_custom_metrics, create_custom_metric, update_custom_metric, export_custom_metrics (YAML response), and import_custom_metrics (multipart YAML upload with dry-run support). --- src/poly/handlers/platform_api.py | 116 ++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/src/poly/handlers/platform_api.py b/src/poly/handlers/platform_api.py index 337f0397..371f5c27 100644 --- a/src/poly/handlers/platform_api.py +++ b/src/poly/handlers/platform_api.py @@ -3,6 +3,7 @@ Copyright PolyAI Limited """ +import io import json import logging import typing as ty @@ -30,6 +31,16 @@ AB_TESTS_URL = "/adk/v1/accounts/{account_id}/projects/{project_id}/ab-tests" AB_TEST_ACTIVE_URL = "/adk/v1/accounts/{account_id}/projects/{project_id}/ab-tests/active" AB_TEST_URL = "/adk/v1/accounts/{account_id}/projects/{project_id}/ab-tests/{ab_test_id}" +CUSTOM_METRICS_URL = "/adk/v1/accounts/{account_id}/projects/{project_id}/custom-metrics" +CUSTOM_METRIC_URL = ( + "/adk/v1/accounts/{account_id}/projects/{project_id}/custom-metrics/{metric_name}" +) +CUSTOM_METRICS_EXPORT_URL = ( + "/adk/v1/accounts/{account_id}/projects/{project_id}/custom-metrics/export" +) +CUSTOM_METRICS_IMPORT_URL = ( + "/adk/v1/accounts/{account_id}/projects/{project_id}/custom-metrics/import" +) # These use public APIs not /adk endpoints PROMOTE_URL = "/v1/agents/{project_id}/deployments/{deployment_id}/promote" ROLLBACK_URL = "/v1/agents/{project_id}/deployments/{deployment_id}/rollback" @@ -1065,3 +1076,108 @@ def trigger_test_run( "branchId": branch_id, } return PlatformAPIHandler.make_request(region, endpoint, "POST", data=data) + + @staticmethod + def export_custom_metrics(region: str, account_id: str, project_id: str) -> dict: + """Export all custom metrics for a project as a YAML-parsed dict. + + Args: + region: The region name. + account_id: The account ID. + project_id: The project ID. + + Returns: + dict: Mapping of metric name to metric definition. + """ + endpoint = CUSTOM_METRICS_EXPORT_URL.format(account_id=account_id, project_id=project_id) + return PlatformAPIHandler.make_request(region, endpoint, "GET", response_format="yaml") + + @staticmethod + def get_custom_metrics(region: str, account_id: str, project_id: str) -> list[dict]: + """List all custom metrics for a project. + + Args: + region: The region name. + account_id: The account ID. + project_id: The project ID. + + Returns: + list[dict]: List of custom metric records. + """ + endpoint = CUSTOM_METRICS_URL.format(account_id=account_id, project_id=project_id) + result = PlatformAPIHandler.make_request(region, endpoint, "GET") + if isinstance(result, list): + return result + return result.get("metrics", result.get("data", [])) + + @staticmethod + def create_custom_metric(region: str, account_id: str, project_id: str, data: dict) -> dict: + """Create a new custom metric. + + Args: + region: The region name. + account_id: The account ID. + project_id: The project ID. + data: Metric payload — name, type, description, expected_values, api. + + Returns: + dict: The created metric record. + """ + endpoint = CUSTOM_METRICS_URL.format(account_id=account_id, project_id=project_id) + return PlatformAPIHandler.make_request(region, endpoint, "POST", data=data) + + @staticmethod + def update_custom_metric( + region: str, + account_id: str, + project_id: str, + metric_name: str, + data: dict, + ) -> dict: + """Update an existing custom metric. + + Args: + region: The region name. + account_id: The account ID. + project_id: The project ID. + metric_name: Name of the metric to update. + data: Fields to update — description, expected_values, active, api. + + Returns: + dict: The updated metric record. + """ + endpoint = CUSTOM_METRIC_URL.format( + account_id=account_id, project_id=project_id, metric_name=metric_name + ) + return PlatformAPIHandler.make_request(region, endpoint, "PATCH", data=data) + + @staticmethod + def import_custom_metrics( + region: str, + account_id: str, + project_id: str, + yaml_content: str, + dry_run: bool = False, + ) -> dict: + """Bulk-import custom metrics from YAML content. + + Args: + region: The region name. + account_id: The account ID. + project_id: The project ID. + yaml_content: Raw YAML string with metric definitions. + dry_run: If True, preview changes without applying. + + Returns: + dict: Import result with metadata.created and metadata.ignored. + """ + endpoint = CUSTOM_METRICS_IMPORT_URL.format(account_id=account_id, project_id=project_id) + params = {"type": "yaml", "dry_run": str(dry_run).lower()} + files = { + "yaml": ( + "metrics.yaml", + io.BytesIO(yaml_content.encode("utf-8")), + "application/x-yaml", + ) + } + return PlatformAPIHandler.make_request(region, endpoint, "POST", params=params, files=files) From 276a105614ed42924c5ea842c99788304cf8aa33 Mon Sep 17 00:00:00 2001 From: bill-parker Date: Fri, 7 Aug 2026 11:38:33 +0100 Subject: [PATCH 03/15] feat: add custom metrics interface layer Add 6 static methods to AgentStudioInterface for custom metrics: get, create, update, export, preview_metrics_import (computes set diff for dry-run), and import. --- src/poly/handlers/interface.py | 139 +++++++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) diff --git a/src/poly/handlers/interface.py b/src/poly/handlers/interface.py index 5bfa2f6b..c9e73c54 100644 --- a/src/poly/handlers/interface.py +++ b/src/poly/handlers/interface.py @@ -1097,3 +1097,142 @@ def trigger_test_run( dict: The created test run response. """ return PlatformAPIHandler.trigger_test_run(region, project_id, test_case_ids, branch_id) + + @staticmethod + def get_custom_metrics( + region: str, + account_id: str, + project_id: str, + ) -> list[dict]: + """List all custom metrics for a project. + + Args: + region: The region name. + account_id: The account ID. + project_id: The project ID. + + Returns: + list[dict]: List of custom metric records. + """ + return PlatformAPIHandler.get_custom_metrics(region, account_id, project_id) + + @staticmethod + def create_custom_metric( + region: str, + account_id: str, + project_id: str, + data: dict, + ) -> dict: + """Create a new custom metric. + + Args: + region: The region name. + account_id: The account ID. + project_id: The project ID. + data: Metric payload — name, type, description, expected_values, api. + + Returns: + dict: The created metric record. + """ + return PlatformAPIHandler.create_custom_metric(region, account_id, project_id, data) + + @staticmethod + def update_custom_metric( + region: str, + account_id: str, + project_id: str, + metric_name: str, + data: dict, + ) -> dict: + """Update an existing custom metric. + + Args: + region: The region name. + account_id: The account ID. + project_id: The project ID. + metric_name: Name of the metric to update. + data: Fields to update — description, expected_values, active, api. + + Returns: + dict: The updated metric record. + """ + return PlatformAPIHandler.update_custom_metric( + region, account_id, project_id, metric_name, data + ) + + @staticmethod + def export_custom_metrics( + region: str, + account_id: str, + project_id: str, + ) -> dict: + """Export all custom metrics as a YAML-parsed dict. + + Args: + region: The region name. + account_id: The account ID. + project_id: The project ID. + + Returns: + dict: Mapping of metric name to metric definition. + """ + return PlatformAPIHandler.export_custom_metrics(region, account_id, project_id) + + @staticmethod + def preview_metrics_import( + region: str, + account_id: str, + project_id: str, + local_metric_names: set[str], + ) -> dict[str, list[str]]: + """Fetch remote metrics and compute what an import would do. + + Compares the local metric names against the remote set to determine + which metrics would be created, skipped, or exist only on the remote. + + Args: + region: The region name. + account_id: The account ID. + project_id: The project ID. + local_metric_names: Set of metric names from the local YAML file. + + Returns: + dict with keys ``would_create``, ``would_skip``, and ``remote_only``, + each a sorted list of metric names. + """ + remote_metrics = PlatformAPIHandler.get_custom_metrics(region, account_id, project_id) + remote_names = {m["name"] for m in remote_metrics if "name" in m} + + would_create = local_metric_names - remote_names + would_skip = local_metric_names & remote_names + remote_only = remote_names - local_metric_names + + return { + "would_create": sorted(would_create), + "would_skip": sorted(would_skip), + "remote_only": sorted(remote_only), + } + + @staticmethod + def import_custom_metrics( + region: str, + account_id: str, + project_id: str, + yaml_content: str, + dry_run: bool = False, + ) -> dict: + """Bulk-import custom metrics from YAML content. + + Args: + region: The region name. + account_id: The account ID. + project_id: The project ID. + yaml_content: Raw YAML string with metric definitions. + dry_run: If True, preview changes without applying. + + Returns: + dict: Import result with metadata.created and metadata.ignored. + """ + return PlatformAPIHandler.import_custom_metrics( + region, account_id, project_id, yaml_content, dry_run + ) From f2f03c6188c7d4e7147980f02c1d835ac3b2a4b8 Mon Sep 17 00:00:00 2001 From: bill-parker Date: Fri, 7 Aug 2026 11:39:33 +0100 Subject: [PATCH 04/15] feat: add poly metrics commands (list, export, add, edit, import) Add MetricsCommand with five subcommands: - list: Rich table display with Name/Type/Active/API/Description - export: YAML to stdout or file - add: create metric via flags or interactive questionary prompts - edit: update description, api, active, expected-values - import: bulk YAML import with --dry-run preview --- src/poly/cli.py | 2 + src/poly/cli_commands/metrics.py | 486 +++++++++++++++++++++++++++++++ src/poly/output/console.py | 40 +++ 3 files changed, 528 insertions(+) create mode 100644 src/poly/cli_commands/metrics.py diff --git a/src/poly/cli.py b/src/poly/cli.py index cd9f4cb0..1552e75b 100644 --- a/src/poly/cli.py +++ b/src/poly/cli.py @@ -18,6 +18,7 @@ from poly.cli_commands.chat import ChatCommand from poly.cli_commands.conversations import ConversationsCommand from poly.cli_commands.deployments import DeploymentsCommand +from poly.cli_commands.metrics import MetricsCommand from poly.cli_commands.project import InitCommand, ProjectCommand, StudioCommand from poly.cli_commands.review import ReviewCommand from poly.cli_commands.sync import ( @@ -51,6 +52,7 @@ ReviewCommand, BranchCommand, DeploymentsCommand, + MetricsCommand, ConversationsCommand, TestingCommand, ChatCommand, diff --git a/src/poly/cli_commands/metrics.py b/src/poly/cli_commands/metrics.py new file mode 100644 index 00000000..33fbd02b --- /dev/null +++ b/src/poly/cli_commands/metrics.py @@ -0,0 +1,486 @@ +"""Metrics command family: list, add, edit, and import custom metrics. + +Copyright PolyAI Limited +""" + +import logging +import os +import sys +from argparse import ArgumentParser, Namespace, RawTextHelpFormatter, _SubParsersAction + +from ruamel.yaml import YAML, YAMLError + +from poly.cli_commands.base import BaseCommand, Parents +from poly.cli_commands.shared import load_project +from poly.handlers.interface import AgentStudioInterface +from poly.output.console import error, plain, print_metrics, success, warning +from poly.output.json_output import json_print + +logger = logging.getLogger(__name__) + +VALID_METRIC_TYPES = ["string", "int", "bool", "float"] + + +def _parse_bool_flag(value: str) -> bool: + """Convert a string flag value to a boolean.""" + if value.lower() in ("true", "1", "yes"): + return True + if value.lower() in ("false", "0", "no"): + return False + raise ValueError(f"Invalid boolean value: {value!r}. Use true/false.") + + +class MetricsCommand(BaseCommand): + """Manage custom metrics in the Agent Studio project.""" + + command = "metrics" + + @classmethod + def add_arguments(cls, subparsers: _SubParsersAction[ArgumentParser], parents: Parents) -> None: + """Register the ``metrics`` subcommand tree.""" + metrics_parser = subparsers.add_parser( + "metrics", + parents=[parents.verbose], + help="Manage custom metrics in the Agent Studio project.", + description=( + "Manage custom metrics in the Agent Studio project.\n\n" + "Examples:\n" + " poly metrics list\n" + " poly metrics add --name SCORE --type int --description 'CSAT Score'\n" + " poly metrics edit CSAT_OFFERED --no-active\n" + " poly metrics import metrics.yaml" + ), + formatter_class=RawTextHelpFormatter, + ) + + metrics_subparsers = metrics_parser.add_subparsers(dest="metrics_subcommand", required=True) + + metrics_subparsers.add_parser( + "list", + parents=[parents.path, parents.json], + help="List all custom metrics in the project.", + description="List all custom metrics for the current project.", + formatter_class=RawTextHelpFormatter, + ) + + export_parser = metrics_subparsers.add_parser( + "export", + parents=[parents.path, parents.json], + help="Export all custom metrics as YAML.", + description=( + "Export all custom metrics as YAML.\n\n" + "Examples:\n" + " poly metrics export\n" + " poly metrics export metrics.yaml\n" + ), + formatter_class=RawTextHelpFormatter, + ) + export_parser.add_argument( + "file", + nargs="?", + default=None, + type=str, + help="Output file path. Prints to stdout when omitted.", + ) + + add_parser = metrics_subparsers.add_parser( + "add", + parents=[parents.path, parents.json], + help="Create a new custom metric.", + description=( + "Create a new custom metric. Required fields prompt\n" + "interactively when omitted.\n\n" + "Examples:\n" + " poly metrics add --name CALL_DURATION --type int" + " --description 'Duration in seconds'\n" + " poly metrics add # interactive mode\n" + ), + formatter_class=RawTextHelpFormatter, + ) + add_parser.add_argument( + "--name", + type=str, + help="Metric name.", + ) + add_parser.add_argument( + "--type", + type=str, + dest="metric_type", + choices=VALID_METRIC_TYPES, + help="Metric value type: string, int, bool, or float.", + ) + add_parser.add_argument( + "--description", + type=str, + default=None, + help="Optional description for the metric.", + ) + add_parser.add_argument( + "--api", + action="store_true", + default=False, + help="Mark as an API metric.", + ) + add_parser.add_argument( + "--expected-values", + type=str, + nargs="+", + default=None, + help="Expected values (only valid for string type).", + ) + + edit_parser = metrics_subparsers.add_parser( + "edit", + parents=[parents.path, parents.json], + help="Update an existing custom metric.", + description=( + "Update an existing custom metric. At least one flag required.\n\n" + "Examples:\n" + " poly metrics edit CARRIER_ID --description 'Carrier handling the shipment'\n" + " poly metrics edit CSAT_OFFERED --active false\n" + " poly metrics edit SCORE --api\n" + ), + formatter_class=RawTextHelpFormatter, + ) + edit_parser.add_argument( + "name", + type=str, + help="Name of the metric to edit.", + ) + edit_parser.add_argument( + "--description", + type=str, + default=None, + help="New description for the metric.", + ) + edit_parser.add_argument( + "--api", + type=_parse_bool_flag, + nargs="?", + const=True, + default=None, + help="Set API flag (true/false). Omit value to set true.", + ) + edit_parser.add_argument( + "--active", + type=_parse_bool_flag, + nargs="?", + const=True, + default=None, + help="Set active state (true/false). Omit value to set true.", + ) + edit_parser.add_argument( + "--expected-values", + type=str, + nargs="+", + default=None, + help="Expected values (only valid for string type).", + ) + + import_parser = metrics_subparsers.add_parser( + "import", + parents=[parents.path, parents.json], + help="Bulk-import metrics from a YAML file.", + description=( + "Bulk-import metrics from a YAML file. Creates metrics that\n" + "don't already exist and skips those that do. Never deletes.\n\n" + "Examples:\n" + " poly metrics import metrics.yaml\n" + " poly metrics import metrics.yaml --dry-run\n" + ), + formatter_class=RawTextHelpFormatter, + ) + import_parser.add_argument( + "file", + type=str, + help="Path to the YAML file to import.", + ) + import_parser.add_argument( + "--dry-run", + action="store_true", + help="Preview what would be created/skipped without making changes.", + ) + + @classmethod + def run(cls, args: Namespace) -> None: + """Dispatch to the matching metrics sub-handler.""" + if args.metrics_subcommand == "list": + cls.metrics_list(args.path, output_json=args.json) + elif args.metrics_subcommand == "export": + cls.metrics_export(args.path, file_path=args.file, output_json=args.json) + elif args.metrics_subcommand == "add": + cls.metrics_add( + args.path, + name=args.name, + metric_type=args.metric_type, + description=args.description, + api=args.api, + expected_values=args.expected_values, + output_json=args.json, + ) + elif args.metrics_subcommand == "edit": + cls.metrics_edit( + args.path, + name=args.name, + description=args.description, + api=args.api, + active=args.active, + expected_values=args.expected_values, + output_json=args.json, + ) + elif args.metrics_subcommand == "import": + cls.metrics_import( + args.path, + file_path=args.file, + dry_run=args.dry_run, + output_json=args.json, + ) + + @classmethod + def metrics_list(cls, base_path: str, output_json: bool = False) -> None: + """List all custom metrics for the project.""" + project = load_project(base_path, output_json=output_json) + metrics = AgentStudioInterface.get_custom_metrics( + project.region, project.account_id, project.project_id + ) + + if output_json: + json_print(metrics) + else: + print_metrics(metrics) + + @classmethod + def metrics_export( + cls, + base_path: str, + file_path: str | None = None, + output_json: bool = False, + ) -> None: + """Export all custom metrics as YAML.""" + project = load_project(base_path, output_json=output_json) + metrics = AgentStudioInterface.export_custom_metrics( + project.region, project.account_id, project.project_id + ) + + if output_json: + json_print(metrics) + return + + ry = YAML() + if file_path: + with open(file_path, "w") as f: + ry.dump(metrics, f) + success(f"Exported metrics to {file_path}") + else: + ry.dump(metrics, sys.stdout) + + @classmethod + def metrics_add( + cls, + base_path: str, + name: str | None = None, + metric_type: str | None = None, + description: str | None = None, + api: bool = False, + expected_values: list[str] | None = None, + output_json: bool = False, + ) -> None: + """Create a new custom metric.""" + project = load_project(base_path, output_json=output_json) + + if name is None: + if output_json: + json_print({"success": False, "error": "--name is required when using --json."}) + sys.exit(1) + import questionary + + name = questionary.text("Metric name:").ask() + if name is None: + sys.exit(1) + name = name.strip() + if not name: + error("Metric name is required.") + sys.exit(1) + + if metric_type is None: + if output_json: + json_print({"success": False, "error": "--type is required when using --json."}) + sys.exit(1) + import questionary + + metric_type = questionary.select("Metric type:", choices=VALID_METRIC_TYPES).ask() + if metric_type is None: + sys.exit(1) + + if description is None and not output_json: + import questionary + + desc = questionary.text("Description (optional):").ask() + if desc is None: + sys.exit(1) + description = desc.strip() or None + + if api is False and not output_json: + import questionary + + api_result = questionary.confirm("API metric?", default=False).ask() + if api_result is None: + sys.exit(1) + api = api_result + + data: dict = {"name": name, "type": metric_type} + if description: + data["description"] = description + if api: + data["api"] = True + if expected_values: + data["expected_values"] = expected_values + + result = AgentStudioInterface.create_custom_metric( + project.region, project.account_id, project.project_id, data + ) + + if output_json: + json_print({"success": True, "metric": result}) + else: + success(f"Created metric {name} ({metric_type})") + + @classmethod + def metrics_edit( + cls, + base_path: str, + name: str, + description: str | None = None, + api: bool | None = None, + active: bool | None = None, + expected_values: list[str] | None = None, + output_json: bool = False, + ) -> None: + """Update an existing custom metric.""" + project = load_project(base_path, output_json=output_json) + + data: dict = {} + if description is not None: + data["description"] = description + if api is not None: + data["api"] = api + if active is not None: + data["active"] = active + if expected_values is not None: + data["expected_values"] = expected_values + + if not data: + msg = "At least one flag is required (--description, --api, --active, etc.)." + if output_json: + json_print({"success": False, "error": msg}) + else: + error(msg) + sys.exit(1) + + result = AgentStudioInterface.update_custom_metric( + project.region, project.account_id, project.project_id, name, data + ) + + if output_json: + json_print({"success": True, "metric": result}) + else: + if data.get("active") is False: + success(f"Deactivated metric {name}") + else: + success(f"Updated metric {name}") + + @classmethod + def metrics_import( + cls, + base_path: str, + file_path: str, + dry_run: bool = False, + output_json: bool = False, + ) -> None: + """Bulk-import metrics from a YAML file.""" + project = load_project(base_path, output_json=output_json) + + if not os.path.exists(file_path): + msg = f"File not found: {file_path}" + if output_json: + json_print({"success": False, "error": msg}) + else: + error(msg) + sys.exit(1) + + with open(file_path) as f: + yaml_content = f.read() + + try: + ry = YAML() + local_metrics = ry.load(yaml_content) or {} + except YAMLError as e: + msg = f"Invalid YAML: {e}" + if output_json: + json_print({"success": False, "error": msg}) + else: + error(msg) + sys.exit(1) + + local_names = set(local_metrics.keys()) + + preview = AgentStudioInterface.preview_metrics_import( + project.region, project.account_id, project.project_id, local_names + ) + + if dry_run: + cls._print_dry_run(preview, output_json) + return + + # Warn about metrics not in the file + if preview["remote_only"] and not output_json: + warning( + f"Metrics on remote but not in file (not deleted):" + f" {', '.join(preview['remote_only'])}" + ) + + import_result = AgentStudioInterface.import_custom_metrics( + project.region, + project.account_id, + project.project_id, + yaml_content, + dry_run=False, + ) + + if output_json: + json_print({"success": True, **import_result}) + else: + metadata = import_result.get("metadata", {}) + created = metadata.get("created", []) + ignored = metadata.get("ignored", []) + + if created: + plain(f"Created: {', '.join(created)}") + if ignored: + plain(f"Skipped (already exist): {', '.join(ignored)}") + + created_count = len(created) + skipped_count = len(ignored) + success(f"Imported {created_count} metrics ({skipped_count} skipped)") + + # Helper function + + @staticmethod + def _print_dry_run( + preview: dict[str, list[str]], + output_json: bool, + ) -> None: + """Display the results of a dry-run import.""" + if output_json: + json_print({"dry_run": True, **preview}) + else: + plain("[dim]Dry run — no changes will be made.[/dim]") + if preview["would_create"]: + plain(f"Would create: {', '.join(preview['would_create'])}") + if preview["would_skip"]: + plain(f"Would skip (already exist): {', '.join(preview['would_skip'])}") + if preview["remote_only"]: + warning( + f"Metrics on remote but not in file (will NOT be deleted):" + f" {', '.join(preview['remote_only'])}" + ) diff --git a/src/poly/output/console.py b/src/poly/output/console.py index 86cfdf89..7b043651 100644 --- a/src/poly/output/console.py +++ b/src/poly/output/console.py @@ -136,6 +136,46 @@ def print_agents(agents: list[dict[str, Any]]) -> None: console.print(table) +def print_metrics(metrics: list[dict[str, Any]]) -> None: + """Print a table of custom metrics. + + Args: + metrics: List of metric dicts from the API. + """ + if not metrics: + plain("No metrics found.") + return + + table = Table(box=None, show_header=True, header_style="bold", padding=(0, 1)) + table.add_column("Name", style="bold yellow", no_wrap=True) + table.add_column("Type", no_wrap=True) + table.add_column("Active", no_wrap=True) + table.add_column("API", no_wrap=True) + table.add_column("Description", max_width=50) + + active_count = 0 + inactive_count = 0 + for m in metrics: + is_active = m.get("active", True) + if is_active: + active_count += 1 + else: + inactive_count += 1 + + desc = m.get("description", "") or "" + + table.add_row( + m.get("name", "—"), + m.get("type", "—"), + "✓" if is_active else "✗", + "✓" if m.get("api", False) else "✗", + desc, + ) + + console.print(table) + console.print(f"\n{len(metrics)} metrics ({active_count} active, {inactive_count} inactive)") + + def print_branches(branches: dict[str, str] | list[str], current_branch: str | None) -> None: """Print branch list with current branch highlighted.""" console.print("[label]Branches:[/label]") From 73af0dfbc66261142bf5cd0205b3dd52feee2550 Mon Sep 17 00:00:00 2001 From: bill-parker Date: Fri, 7 Aug 2026 11:39:51 +0100 Subject: [PATCH 05/15] fix: work around server ignoring api flag on metric create The custom_metrics create route in jupiter_api hardcodes api=False (routes.py:128), ignoring the client-supplied value. The Agent Studio UI also does not set the api flag on create. As a workaround, when the user passes --api, the CLI issues a follow-up PATCH to set api=True after creation. --- src/poly/cli_commands/metrics.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/poly/cli_commands/metrics.py b/src/poly/cli_commands/metrics.py index 33fbd02b..72686f8e 100644 --- a/src/poly/cli_commands/metrics.py +++ b/src/poly/cli_commands/metrics.py @@ -340,6 +340,12 @@ def metrics_add( project.region, project.account_id, project.project_id, data ) + # The server ignores the api flag on create, so follow up with an edit + if api: + result = AgentStudioInterface.update_custom_metric( + project.region, project.account_id, project.project_id, name, {"api": True} + ) + if output_json: json_print({"success": True, "metric": result}) else: From 3454c4a7f708cdf989d38a6b98068adf9360972a Mon Sep 17 00:00:00 2001 From: bill-parker Date: Fri, 7 Aug 2026 11:40:00 +0100 Subject: [PATCH 06/15] test: add unit tests for metrics CLI commands 30 tests across 8 test classes covering: - _parse_bool_flag edge cases - list, export, add, edit, import subcommands - dry-run preview display - api flag workaround (create + follow-up PATCH) - VALID_METRIC_TYPES constant --- src/poly/tests/metrics_test.py | 503 +++++++++++++++++++++++++++++++++ 1 file changed, 503 insertions(+) create mode 100644 src/poly/tests/metrics_test.py diff --git a/src/poly/tests/metrics_test.py b/src/poly/tests/metrics_test.py new file mode 100644 index 00000000..5796b68e --- /dev/null +++ b/src/poly/tests/metrics_test.py @@ -0,0 +1,503 @@ +"""Tests for the metrics CLI commands. + +Copyright PolyAI Limited +""" + +import unittest +from unittest.mock import MagicMock, patch + +from poly.cli_commands.metrics import VALID_METRIC_TYPES, MetricsCommand, _parse_bool_flag + + +class ParseBoolFlagTest(unittest.TestCase): + """Tests for _parse_bool_flag helper.""" + + def test_true_values(self): + """Accepts 'true', '1', 'yes' (case-insensitive) as True.""" + for val in ("true", "True", "TRUE", "1", "yes", "Yes", "YES"): + self.assertTrue(_parse_bool_flag(val), f"Expected True for {val!r}") + + def test_false_values(self): + """Accepts 'false', '0', 'no' (case-insensitive) as False.""" + for val in ("false", "False", "FALSE", "0", "no", "No", "NO"): + self.assertFalse(_parse_bool_flag(val), f"Expected False for {val!r}") + + def test_invalid_value_raises(self): + """Raises ValueError for unrecognized strings.""" + with self.assertRaises(ValueError) as ctx: + _parse_bool_flag("maybe") + self.assertIn("maybe", str(ctx.exception)) + + def test_empty_string_raises(self): + """Raises ValueError for an empty string.""" + with self.assertRaises(ValueError): + _parse_bool_flag("") + + +class MetricsListTest(unittest.TestCase): + """Tests for MetricsCommand.metrics_list.""" + + @patch("poly.cli_commands.metrics.print_metrics") + @patch("poly.cli_commands.metrics.AgentStudioInterface.get_custom_metrics") + @patch("poly.cli_commands.metrics.load_project") + def test_list_calls_print_metrics(self, mock_load, mock_get, mock_print): + """metrics_list fetches metrics and prints them in table mode.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + mock_get.return_value = [{"name": "SCORE", "type": "int"}] + + MetricsCommand.metrics_list("/fake/path", output_json=False) + + mock_get.assert_called_once_with("us", "acc1", "proj1") + mock_print.assert_called_once_with([{"name": "SCORE", "type": "int"}]) + + @patch("poly.cli_commands.metrics.json_print") + @patch("poly.cli_commands.metrics.AgentStudioInterface.get_custom_metrics") + @patch("poly.cli_commands.metrics.load_project") + def test_list_json_output(self, mock_load, mock_get, mock_json): + """metrics_list uses json_print when output_json=True.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + metrics = [{"name": "SCORE"}] + mock_get.return_value = metrics + + MetricsCommand.metrics_list("/fake/path", output_json=True) + + mock_json.assert_called_once_with(metrics) + + +class MetricsExportTest(unittest.TestCase): + """Tests for MetricsCommand.metrics_export.""" + + @patch("poly.cli_commands.metrics.json_print") + @patch("poly.cli_commands.metrics.AgentStudioInterface.export_custom_metrics") + @patch("poly.cli_commands.metrics.load_project") + def test_export_json_output(self, mock_load, mock_export, mock_json): + """In JSON mode, export passes the dict straight to json_print.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + data = {"SCORE": {"type": "int"}, "STATUS": {"type": "string"}} + mock_export.return_value = data + + MetricsCommand.metrics_export("/fake/path", output_json=True) + + mock_json.assert_called_once_with(data) + + @patch("poly.cli_commands.metrics.AgentStudioInterface.export_custom_metrics") + @patch("poly.cli_commands.metrics.load_project") + def test_export_to_stdout(self, mock_load, mock_export): + """Without a file path, YAML is dumped to stdout.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + mock_export.return_value = {"SCORE": {"type": "int"}} + + with patch("poly.cli_commands.metrics.YAML") as mock_yaml_cls: + mock_ry = MagicMock() + mock_yaml_cls.return_value = mock_ry + MetricsCommand.metrics_export("/fake/path", file_path=None, output_json=False) + + import sys + + mock_ry.dump.assert_called_once_with({"SCORE": {"type": "int"}}, sys.stdout) + + @patch("poly.cli_commands.metrics.success") + @patch("poly.cli_commands.metrics.AgentStudioInterface.export_custom_metrics") + @patch("poly.cli_commands.metrics.load_project") + def test_export_to_file(self, mock_load, mock_export, mock_success): + """With a file path, YAML is written to file and success message shown.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + mock_export.return_value = {"SCORE": {"type": "int"}} + + with ( + patch("poly.cli_commands.metrics.YAML") as mock_yaml_cls, + patch("builtins.open", unittest.mock.mock_open()) as mock_file, + ): + mock_ry = MagicMock() + mock_yaml_cls.return_value = mock_ry + MetricsCommand.metrics_export("/fake/path", file_path="out.yaml", output_json=False) + + mock_file.assert_called_once_with("out.yaml", "w") + mock_ry.dump.assert_called_once() + mock_success.assert_called_once() + self.assertIn("out.yaml", mock_success.call_args[0][0]) + + @patch("poly.cli_commands.metrics.AgentStudioInterface.export_custom_metrics") + @patch("poly.cli_commands.metrics.load_project") + def test_export_empty_metrics(self, mock_load, mock_export): + """Export with empty dict still dumps without error.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + mock_export.return_value = {} + + with patch("poly.cli_commands.metrics.YAML") as mock_yaml_cls: + mock_ry = MagicMock() + mock_yaml_cls.return_value = mock_ry + MetricsCommand.metrics_export("/fake/path", output_json=False) + + mock_ry.dump.assert_called_once_with({}, unittest.mock.ANY) + + +class MetricsAddTest(unittest.TestCase): + """Tests for MetricsCommand.metrics_add.""" + + @patch("poly.cli_commands.metrics.success") + @patch("poly.cli_commands.metrics.AgentStudioInterface.update_custom_metric") + @patch("poly.cli_commands.metrics.AgentStudioInterface.create_custom_metric") + @patch("poly.cli_commands.metrics.load_project") + def test_add_with_all_args(self, mock_load, mock_create, mock_update, mock_success): + """Non-interactive add passes all fields to create_custom_metric.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + mock_create.return_value = {"name": "SCORE", "type": "int"} + mock_update.return_value = {"name": "SCORE", "type": "int", "api": True} + + MetricsCommand.metrics_add( + "/fake/path", + name="SCORE", + metric_type="int", + description="CSAT Score", + api=True, + expected_values=None, + output_json=False, + ) + + mock_create.assert_called_once_with( + "us", + "acc1", + "proj1", + {"name": "SCORE", "type": "int", "description": "CSAT Score", "api": True}, + ) + mock_update.assert_called_once_with("us", "acc1", "proj1", "SCORE", {"api": True}) + mock_success.assert_called_once() + + @patch("poly.cli_commands.metrics.json_print") + @patch("poly.cli_commands.metrics.AgentStudioInterface.update_custom_metric") + @patch("poly.cli_commands.metrics.AgentStudioInterface.create_custom_metric") + @patch("poly.cli_commands.metrics.load_project") + def test_add_without_api_skips_update(self, mock_load, mock_create, mock_update, mock_json): + """When api=False, no follow-up update call is made.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + mock_create.return_value = {"name": "SCORE", "type": "int"} + + MetricsCommand.metrics_add( + "/fake/path", + name="SCORE", + metric_type="int", + api=False, + output_json=True, + ) + + mock_create.assert_called_once() + mock_update.assert_not_called() + + @patch("poly.cli_commands.metrics.json_print") + @patch("poly.cli_commands.metrics.AgentStudioInterface.create_custom_metric") + @patch("poly.cli_commands.metrics.load_project") + def test_add_passes_expected_values(self, mock_load, mock_create, mock_json): + """Expected values are included in the data dict when provided.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + mock_create.return_value = {} + + MetricsCommand.metrics_add( + "/fake/path", + name="STATUS", + metric_type="string", + description=None, + api=False, + expected_values=["open", "closed"], + output_json=True, + ) + + data = mock_create.call_args[0][3] + self.assertEqual(data["expected_values"], ["open", "closed"]) + + @patch("poly.cli_commands.metrics.json_print") + @patch("poly.cli_commands.metrics.load_project") + def test_add_json_error_when_name_missing(self, mock_load, mock_json): + """In JSON mode, missing --name prints an error and exits.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + + with self.assertRaises(SystemExit) as ctx: + MetricsCommand.metrics_add( + "/fake/path", + name=None, + metric_type="int", + output_json=True, + ) + + self.assertEqual(ctx.exception.code, 1) + mock_json.assert_called_once() + printed = mock_json.call_args[0][0] + self.assertFalse(printed["success"]) + self.assertIn("--name", printed["error"]) + + @patch("poly.cli_commands.metrics.json_print") + @patch("poly.cli_commands.metrics.load_project") + def test_add_json_error_when_type_missing(self, mock_load, mock_json): + """In JSON mode, missing --type prints an error and exits.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + + with self.assertRaises(SystemExit) as ctx: + MetricsCommand.metrics_add( + "/fake/path", + name="SCORE", + metric_type=None, + output_json=True, + ) + + self.assertEqual(ctx.exception.code, 1) + printed = mock_json.call_args[0][0] + self.assertIn("--type", printed["error"]) + + @patch("poly.cli_commands.metrics.json_print") + @patch("poly.cli_commands.metrics.AgentStudioInterface.create_custom_metric") + @patch("poly.cli_commands.metrics.load_project") + def test_add_json_output_on_success(self, mock_load, mock_create, mock_json): + """In JSON mode, successful add prints success with the metric.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + result = {"name": "SCORE", "type": "int"} + mock_create.return_value = result + + MetricsCommand.metrics_add( + "/fake/path", + name="SCORE", + metric_type="int", + output_json=True, + ) + + mock_json.assert_called_once_with({"success": True, "metric": result}) + + +class MetricsEditTest(unittest.TestCase): + """Tests for MetricsCommand.metrics_edit.""" + + @patch("poly.cli_commands.metrics.success") + @patch("poly.cli_commands.metrics.AgentStudioInterface.update_custom_metric") + @patch("poly.cli_commands.metrics.load_project") + def test_edit_with_description(self, mock_load, mock_update, mock_success): + """Editing description passes it through to update_custom_metric.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + mock_update.return_value = {} + + MetricsCommand.metrics_edit( + "/fake/path", name="SCORE", description="New desc", output_json=False + ) + + mock_update.assert_called_once_with( + "us", "acc1", "proj1", "SCORE", {"description": "New desc"} + ) + + @patch("poly.cli_commands.metrics.success") + @patch("poly.cli_commands.metrics.AgentStudioInterface.update_custom_metric") + @patch("poly.cli_commands.metrics.load_project") + def test_edit_deactivate_metric(self, mock_load, mock_update, mock_success): + """Setting active=False prints 'Deactivated' message.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + mock_update.return_value = {} + + MetricsCommand.metrics_edit("/fake/path", name="SCORE", active=False, output_json=False) + + mock_update.assert_called_once_with("us", "acc1", "proj1", "SCORE", {"active": False}) + mock_success.assert_called_once() + self.assertIn("Deactivated", mock_success.call_args[0][0]) + + @patch("poly.cli_commands.metrics.success") + @patch("poly.cli_commands.metrics.AgentStudioInterface.update_custom_metric") + @patch("poly.cli_commands.metrics.load_project") + def test_edit_multiple_flags(self, mock_load, mock_update, mock_success): + """Multiple flags are combined into one update call.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + mock_update.return_value = {} + + MetricsCommand.metrics_edit( + "/fake/path", + name="SCORE", + description="Updated", + api=True, + active=True, + output_json=False, + ) + + data = mock_update.call_args[0][4] + self.assertEqual(data["description"], "Updated") + self.assertTrue(data["api"]) + self.assertTrue(data["active"]) + + @patch("poly.cli_commands.metrics.error") + @patch("poly.cli_commands.metrics.load_project") + def test_edit_no_flags_exits(self, mock_load, mock_error): + """Exits with error when no update flags are provided.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + + with self.assertRaises(SystemExit) as ctx: + MetricsCommand.metrics_edit("/fake/path", name="SCORE", output_json=False) + + self.assertEqual(ctx.exception.code, 1) + mock_error.assert_called_once() + + @patch("poly.cli_commands.metrics.json_print") + @patch("poly.cli_commands.metrics.load_project") + def test_edit_no_flags_json_exits(self, mock_load, mock_json): + """In JSON mode, no flags prints error JSON and exits.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + + with self.assertRaises(SystemExit): + MetricsCommand.metrics_edit("/fake/path", name="SCORE", output_json=True) + + printed = mock_json.call_args[0][0] + self.assertFalse(printed["success"]) + + @patch("poly.cli_commands.metrics.json_print") + @patch("poly.cli_commands.metrics.AgentStudioInterface.update_custom_metric") + @patch("poly.cli_commands.metrics.load_project") + def test_edit_json_output(self, mock_load, mock_update, mock_json): + """In JSON mode, successful edit prints success with the metric.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + result = {"name": "SCORE", "active": True} + mock_update.return_value = result + + MetricsCommand.metrics_edit("/fake/path", name="SCORE", active=True, output_json=True) + + mock_json.assert_called_once_with({"success": True, "metric": result}) + + +class MetricsImportTest(unittest.TestCase): + """Tests for MetricsCommand.metrics_import.""" + + @patch("poly.cli_commands.metrics.error") + @patch("poly.cli_commands.metrics.load_project") + def test_import_file_not_found(self, mock_load, mock_error): + """Exits with error when the import file does not exist.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + + with self.assertRaises(SystemExit) as ctx: + MetricsCommand.metrics_import( + "/fake/path", file_path="/nonexistent/metrics.yaml", output_json=False + ) + + self.assertEqual(ctx.exception.code, 1) + mock_error.assert_called_once() + self.assertIn("File not found", mock_error.call_args[0][0]) + + @patch("poly.cli_commands.metrics.json_print") + @patch("poly.cli_commands.metrics.load_project") + def test_import_file_not_found_json(self, mock_load, mock_json): + """In JSON mode, missing file prints error JSON and exits.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + + with self.assertRaises(SystemExit): + MetricsCommand.metrics_import( + "/fake/path", file_path="/nonexistent/metrics.yaml", output_json=True + ) + + printed = mock_json.call_args[0][0] + self.assertFalse(printed["success"]) + + @patch("builtins.open", unittest.mock.mock_open(read_data="{{invalid")) + @patch("os.path.exists", return_value=True) + @patch("poly.cli_commands.metrics.error") + @patch("poly.cli_commands.metrics.load_project") + def test_import_invalid_yaml(self, mock_load, mock_error, mock_exists): + """Exits with error when YAML parsing fails.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + + with self.assertRaises(SystemExit) as ctx: + MetricsCommand.metrics_import("/fake/path", file_path="bad.yaml", output_json=False) + + self.assertEqual(ctx.exception.code, 1) + + @patch("builtins.open", unittest.mock.mock_open(read_data="SCORE:\n type: int\n")) + @patch("os.path.exists", return_value=True) + @patch("poly.cli_commands.metrics.AgentStudioInterface.import_custom_metrics") + @patch("poly.cli_commands.metrics.AgentStudioInterface.preview_metrics_import") + @patch("poly.cli_commands.metrics.load_project") + def test_import_success(self, mock_load, mock_preview, mock_import, mock_exists): + """Successful import calls import_custom_metrics and prints summary.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + mock_preview.return_value = {"remote_only": []} + mock_import.return_value = { + "metadata": {"created": ["SCORE"], "ignored": []}, + } + + with patch("poly.cli_commands.metrics.success"), patch("poly.cli_commands.metrics.plain"): + MetricsCommand.metrics_import("/fake/path", file_path="metrics.yaml", output_json=False) + + mock_import.assert_called_once() + # Verify dry_run=False was passed + self.assertFalse(mock_import.call_args[1]["dry_run"]) + + +class PrintDryRunTest(unittest.TestCase): + """Tests for MetricsCommand._print_dry_run static method.""" + + @patch("poly.cli_commands.metrics.json_print") + def test_dry_run_json_output(self, mock_json): + """JSON dry-run output includes would_create, would_skip, remote_only.""" + preview = { + "would_create": ["A", "C"], + "would_skip": ["B"], + "remote_only": ["D"], + } + + MetricsCommand._print_dry_run(preview, output_json=True) + + result = mock_json.call_args[0][0] + self.assertTrue(result["dry_run"]) + self.assertEqual(result["would_create"], ["A", "C"]) + self.assertEqual(result["would_skip"], ["B"]) + self.assertEqual(result["remote_only"], ["D"]) + + @patch("poly.cli_commands.metrics.warning") + @patch("poly.cli_commands.metrics.plain") + def test_dry_run_text_output(self, mock_plain, mock_warning): + """Text dry-run shows create, skip, and remote-only warnings.""" + preview = { + "would_create": ["NEW_METRIC"], + "would_skip": ["EXISTING"], + "remote_only": ["REMOTE_ONLY"], + } + + MetricsCommand._print_dry_run(preview, output_json=False) + + calls = [c[0][0] for c in mock_plain.call_args_list] + self.assertTrue(any("Would create" in c for c in calls)) + self.assertTrue(any("Would skip" in c for c in calls)) + mock_warning.assert_called_once() + + @patch("poly.cli_commands.metrics.plain") + def test_dry_run_empty_preview(self, mock_plain): + """With empty lists, only the dim header is printed.""" + preview = {"would_create": [], "would_skip": [], "remote_only": []} + + MetricsCommand._print_dry_run(preview, output_json=False) + + # Only the dim header should be printed + self.assertEqual(mock_plain.call_count, 1) + + +class ValidMetricTypesTest(unittest.TestCase): + """Tests for the VALID_METRIC_TYPES constant.""" + + def test_contains_expected_types(self): + """All expected metric types are present.""" + self.assertEqual(VALID_METRIC_TYPES, ["string", "int", "bool", "float"]) + + +if __name__ == "__main__": + unittest.main() From 7b0f7289b5fc079f412f45e146d6435fc017c1e3 Mon Sep 17 00:00:00 2001 From: bill-parker Date: Fri, 7 Aug 2026 12:23:01 +0100 Subject: [PATCH 07/15] feat: add interactive edit mode for metrics --- src/poly/cli_commands/metrics.py | 95 ++++++++++++++++++++++++-- src/poly/tests/metrics_test.py | 112 +++++++++++++++++++++++++++++-- 2 files changed, 195 insertions(+), 12 deletions(-) diff --git a/src/poly/cli_commands/metrics.py b/src/poly/cli_commands/metrics.py index 72686f8e..181e9b72 100644 --- a/src/poly/cli_commands/metrics.py +++ b/src/poly/cli_commands/metrics.py @@ -375,12 +375,11 @@ def metrics_edit( if expected_values is not None: data["expected_values"] = expected_values - if not data: + if not data and not output_json: + data = cls._interactive_edit(project, name) + elif not data: msg = "At least one flag is required (--description, --api, --active, etc.)." - if output_json: - json_print({"success": False, "error": msg}) - else: - error(msg) + json_print({"success": False, "error": msg}) sys.exit(1) result = AgentStudioInterface.update_custom_metric( @@ -395,6 +394,92 @@ def metrics_edit( else: success(f"Updated metric {name}") + @classmethod + def _interactive_edit(cls, project: object, name: str) -> dict: + """Prompt the user to select and edit metric fields interactively. + + Args: + project: The loaded AgentStudioProject. + name: Name of the metric to edit. + + Returns: + A dict of fields to update. + """ + import questionary + + metrics = AgentStudioInterface.get_custom_metrics( + project.region, + project.account_id, + project.project_id, # type: ignore[attr-defined] + ) + + metric = next((m for m in metrics if m.get("name") == name), None) + if metric is None: + error(f"Metric {name!r} not found.") + sys.exit(1) + + # Display current values + plain(f"\n[bold]Current values for {name}:[/bold]") + plain(f" name: {metric.get('name', '—')}") + plain(f" type: {metric.get('type', '—')}") + plain(f" description: {metric.get('description') or '—'}") + plain(f" api: {metric.get('api', False)}") + plain(f" active: {metric.get('active', True)}") + ev = metric.get("expected_values") or [] + plain(f" expected_values: {' '.join(ev) if ev else '—'}") + plain("") + + fields = questionary.checkbox( + "Which fields do you want to edit?", + choices=["description", "api", "active", "expected_values"], + ).ask() + if fields is None: + sys.exit(1) + if not fields: + error("No fields selected.") + sys.exit(1) + + data: dict = {} + + if "description" in fields: + val = questionary.text( + "description:", + default=metric.get("description") or "", + ).ask() + if val is None: + sys.exit(1) + data["description"] = val + + if "api" in fields: + val = questionary.confirm( + "api:", + default=metric.get("api", False), + ).ask() + if val is None: + sys.exit(1) + data["api"] = val + + if "active" in fields: + val = questionary.confirm( + "active:", + default=metric.get("active", True), + ).ask() + if val is None: + sys.exit(1) + data["active"] = val + + if "expected_values" in fields: + current = metric.get("expected_values") or [] + val = questionary.text( + "expected_values (space-separated):", + default=" ".join(current), + ).ask() + if val is None: + sys.exit(1) + data["expected_values"] = val.split() if val.strip() else [] + + return data + @classmethod def metrics_import( cls, diff --git a/src/poly/tests/metrics_test.py b/src/poly/tests/metrics_test.py index 5796b68e..f2b71a7f 100644 --- a/src/poly/tests/metrics_test.py +++ b/src/poly/tests/metrics_test.py @@ -332,18 +332,20 @@ def test_edit_multiple_flags(self, mock_load, mock_update, mock_success): self.assertTrue(data["api"]) self.assertTrue(data["active"]) - @patch("poly.cli_commands.metrics.error") + @patch("poly.cli_commands.metrics.MetricsCommand._interactive_edit") + @patch("poly.cli_commands.metrics.AgentStudioInterface.update_custom_metric") @patch("poly.cli_commands.metrics.load_project") - def test_edit_no_flags_exits(self, mock_load, mock_error): - """Exits with error when no update flags are provided.""" + def test_edit_no_flags_triggers_interactive(self, mock_load, mock_update, mock_interactive): + """Enters interactive mode when no flags are provided and not JSON.""" project = MagicMock(region="us", account_id="acc1", project_id="proj1") mock_load.return_value = project + mock_interactive.return_value = {"description": "New desc"} + mock_update.return_value = {"name": "SCORE", "description": "New desc"} - with self.assertRaises(SystemExit) as ctx: - MetricsCommand.metrics_edit("/fake/path", name="SCORE", output_json=False) + MetricsCommand.metrics_edit("/fake/path", name="SCORE", output_json=False) - self.assertEqual(ctx.exception.code, 1) - mock_error.assert_called_once() + mock_interactive.assert_called_once_with(project, "SCORE") + mock_update.assert_called_once() @patch("poly.cli_commands.metrics.json_print") @patch("poly.cli_commands.metrics.load_project") @@ -373,6 +375,102 @@ def test_edit_json_output(self, mock_load, mock_update, mock_json): mock_json.assert_called_once_with({"success": True, "metric": result}) +class InteractiveEditTest(unittest.TestCase): + """Tests for MetricsCommand._interactive_edit.""" + + SAMPLE_METRICS = [ + { + "name": "SCORE", + "type": "int", + "description": "CSAT score", + "api": False, + "active": True, + "expected_values": [], + }, + ] + + @patch("poly.cli_commands.metrics.AgentStudioInterface.get_custom_metrics") + @patch("poly.cli_commands.metrics.error") + def test_metric_not_found_exits(self, mock_error, mock_get): + """Exits with error when the named metric does not exist.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_get.return_value = self.SAMPLE_METRICS + + with self.assertRaises(SystemExit) as ctx: + MetricsCommand._interactive_edit(project, "NONEXISTENT") + + self.assertEqual(ctx.exception.code, 1) + mock_error.assert_called_once() + + @patch("questionary.checkbox") + @patch("poly.cli_commands.metrics.AgentStudioInterface.get_custom_metrics") + def test_checkbox_cancel_exits(self, mock_get, mock_checkbox): + """Exits when user cancels field selection.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_get.return_value = self.SAMPLE_METRICS + mock_checkbox.return_value.ask.return_value = None + + with self.assertRaises(SystemExit) as ctx: + MetricsCommand._interactive_edit(project, "SCORE") + + self.assertEqual(ctx.exception.code, 1) + + @patch("questionary.checkbox") + @patch("poly.cli_commands.metrics.AgentStudioInterface.get_custom_metrics") + def test_no_fields_selected_exits(self, mock_get, mock_checkbox): + """Exits when user selects no fields.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_get.return_value = self.SAMPLE_METRICS + mock_checkbox.return_value.ask.return_value = [] + + with self.assertRaises(SystemExit) as ctx: + MetricsCommand._interactive_edit(project, "SCORE") + + self.assertEqual(ctx.exception.code, 1) + + @patch("questionary.text") + @patch("questionary.checkbox") + @patch("poly.cli_commands.metrics.AgentStudioInterface.get_custom_metrics") + def test_edit_description(self, mock_get, mock_checkbox, mock_text): + """Returns updated description when user edits it.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_get.return_value = self.SAMPLE_METRICS + mock_checkbox.return_value.ask.return_value = ["description"] + mock_text.return_value.ask.return_value = "New description" + + result = MetricsCommand._interactive_edit(project, "SCORE") + + self.assertEqual(result, {"description": "New description"}) + + @patch("questionary.confirm") + @patch("questionary.checkbox") + @patch("poly.cli_commands.metrics.AgentStudioInterface.get_custom_metrics") + def test_edit_api_and_active(self, mock_get, mock_checkbox, mock_confirm): + """Returns updated api and active flags.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_get.return_value = self.SAMPLE_METRICS + mock_checkbox.return_value.ask.return_value = ["api", "active"] + mock_confirm.return_value.ask.side_effect = [True, False] + + result = MetricsCommand._interactive_edit(project, "SCORE") + + self.assertEqual(result, {"api": True, "active": False}) + + @patch("questionary.text") + @patch("questionary.checkbox") + @patch("poly.cli_commands.metrics.AgentStudioInterface.get_custom_metrics") + def test_edit_expected_values(self, mock_get, mock_checkbox, mock_text): + """Parses space-separated expected values.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_get.return_value = self.SAMPLE_METRICS + mock_checkbox.return_value.ask.return_value = ["expected_values"] + mock_text.return_value.ask.return_value = "low medium high" + + result = MetricsCommand._interactive_edit(project, "SCORE") + + self.assertEqual(result, {"expected_values": ["low", "medium", "high"]}) + + class MetricsImportTest(unittest.TestCase): """Tests for MetricsCommand.metrics_import.""" From b2c64db4cb2f4f3804466c332c8fe062669e2d6c Mon Sep 17 00:00:00 2001 From: bill-parker Date: Fri, 7 Aug 2026 12:27:42 +0100 Subject: [PATCH 08/15] fix: handle dict items in import response metadata The server returns created/ignored items as dicts with name and message fields, not plain strings. Extract the name before joining for display. --- src/poly/cli_commands/metrics.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/poly/cli_commands/metrics.py b/src/poly/cli_commands/metrics.py index 181e9b72..381fcffc 100644 --- a/src/poly/cli_commands/metrics.py +++ b/src/poly/cli_commands/metrics.py @@ -545,10 +545,14 @@ def metrics_import( created = metadata.get("created", []) ignored = metadata.get("ignored", []) + def _item_name(item: dict[str, str] | str) -> str: + """Extract name from a metadata item (dict or plain string).""" + return item["name"] if isinstance(item, dict) else item + if created: - plain(f"Created: {', '.join(created)}") + plain(f"Created: {', '.join(_item_name(i) for i in created)}") if ignored: - plain(f"Skipped (already exist): {', '.join(ignored)}") + plain(f"Skipped (already exist): {', '.join(_item_name(i) for i in ignored)}") created_count = len(created) skipped_count = len(ignored) From a603cc3b7cf67cfb21ddc95b23df39ed9523d25b Mon Sep 17 00:00:00 2001 From: bill-parker Date: Fri, 7 Aug 2026 12:28:32 +0100 Subject: [PATCH 09/15] fix: show friendly errors for duplicate and missing metrics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Catch requests.HTTPError on create (409 → "already exists") and edit (404 → "not found") instead of showing raw HTTP errors. --- src/poly/cli_commands/metrics.py | 45 ++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/src/poly/cli_commands/metrics.py b/src/poly/cli_commands/metrics.py index 381fcffc..d70a860b 100644 --- a/src/poly/cli_commands/metrics.py +++ b/src/poly/cli_commands/metrics.py @@ -8,6 +8,7 @@ import sys from argparse import ArgumentParser, Namespace, RawTextHelpFormatter, _SubParsersAction +import requests from ruamel.yaml import YAML, YAMLError from poly.cli_commands.base import BaseCommand, Parents @@ -336,16 +337,27 @@ def metrics_add( if expected_values: data["expected_values"] = expected_values - result = AgentStudioInterface.create_custom_metric( - project.region, project.account_id, project.project_id, data - ) - - # The server ignores the api flag on create, so follow up with an edit - if api: - result = AgentStudioInterface.update_custom_metric( - project.region, project.account_id, project.project_id, name, {"api": True} + try: + result = AgentStudioInterface.create_custom_metric( + project.region, project.account_id, project.project_id, data ) + # The server ignores the api flag on create, so follow up with an edit + if api: + result = AgentStudioInterface.update_custom_metric( + project.region, project.account_id, project.project_id, name, {"api": True} + ) + except requests.HTTPError as e: + if e.response is not None and e.response.status_code == 409: + msg = f"Metric '{name}' already exists." + else: + msg = f"Failed to create metric: {e.response.text if e.response else e}" + if output_json: + json_print({"success": False, "error": msg}) + else: + error(msg) + sys.exit(1) + if output_json: json_print({"success": True, "metric": result}) else: @@ -382,9 +394,20 @@ def metrics_edit( json_print({"success": False, "error": msg}) sys.exit(1) - result = AgentStudioInterface.update_custom_metric( - project.region, project.account_id, project.project_id, name, data - ) + try: + result = AgentStudioInterface.update_custom_metric( + project.region, project.account_id, project.project_id, name, data + ) + except requests.HTTPError as e: + if e.response is not None and e.response.status_code == 404: + msg = f"Metric '{name}' not found." + else: + msg = f"Failed to update metric: {e.response.text if e.response else e}" + if output_json: + json_print({"success": False, "error": msg}) + else: + error(msg) + sys.exit(1) if output_json: json_print({"success": True, "metric": result}) From 931f58384d509eed6aebeb3e2932668fea120d12 Mon Sep 17 00:00:00 2001 From: bill-parker Date: Fri, 7 Aug 2026 13:54:32 +0100 Subject: [PATCH 10/15] fix: hide expected values from non-string metrics --- src/poly/cli_commands/metrics.py | 34 ++++++++++-- src/poly/tests/metrics_test.py | 90 ++++++++++++++++++++++++++++++-- 2 files changed, 118 insertions(+), 6 deletions(-) diff --git a/src/poly/cli_commands/metrics.py b/src/poly/cli_commands/metrics.py index d70a860b..997cf284 100644 --- a/src/poly/cli_commands/metrics.py +++ b/src/poly/cli_commands/metrics.py @@ -329,6 +329,14 @@ def metrics_add( sys.exit(1) api = api_result + if expected_values and metric_type != "string": + msg = "--expected-values is only valid for string metrics." + if output_json: + json_print({"success": False, "error": msg}) + else: + error(msg) + sys.exit(1) + data: dict = {"name": name, "type": metric_type} if description: data["description"] = description @@ -377,6 +385,19 @@ def metrics_edit( """Update an existing custom metric.""" project = load_project(base_path, output_json=output_json) + if expected_values is not None: + metrics = AgentStudioInterface.get_custom_metrics( + project.region, project.account_id, project.project_id + ) + metric = next((m for m in metrics if m.get("name") == name), None) + if metric and metric.get("type") != "string": + msg = "--expected-values is only valid for string metrics." + if output_json: + json_print({"success": False, "error": msg}) + else: + error(msg) + sys.exit(1) + data: dict = {} if description is not None: data["description"] = description @@ -441,6 +462,8 @@ def _interactive_edit(cls, project: object, name: str) -> dict: error(f"Metric {name!r} not found.") sys.exit(1) + is_string = metric.get("type") == "string" + # Display current values plain(f"\n[bold]Current values for {name}:[/bold]") plain(f" name: {metric.get('name', '—')}") @@ -448,13 +471,18 @@ def _interactive_edit(cls, project: object, name: str) -> dict: plain(f" description: {metric.get('description') or '—'}") plain(f" api: {metric.get('api', False)}") plain(f" active: {metric.get('active', True)}") - ev = metric.get("expected_values") or [] - plain(f" expected_values: {' '.join(ev) if ev else '—'}") + if is_string: + ev = metric.get("expected_values") or [] + plain(f" expected_values: {' '.join(ev) if ev else '—'}") plain("") + choices = ["description", "api", "active"] + if is_string: + choices.append("expected_values") + fields = questionary.checkbox( "Which fields do you want to edit?", - choices=["description", "api", "active", "expected_values"], + choices=choices, ).ask() if fields is None: sys.exit(1) diff --git a/src/poly/tests/metrics_test.py b/src/poly/tests/metrics_test.py index f2b71a7f..8edda6c0 100644 --- a/src/poly/tests/metrics_test.py +++ b/src/poly/tests/metrics_test.py @@ -254,6 +254,48 @@ def test_add_json_error_when_type_missing(self, mock_load, mock_json): printed = mock_json.call_args[0][0] self.assertIn("--type", printed["error"]) + @patch("poly.cli_commands.metrics.error") + @patch("poly.cli_commands.metrics.load_project") + def test_add_expected_values_rejected_for_non_string(self, mock_load, mock_error): + """Expected values are rejected for non-string metric types.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + + with self.assertRaises(SystemExit) as ctx: + MetricsCommand.metrics_add( + "/fake/path", + name="SCORE", + metric_type="int", + description="desc", + api=True, + expected_values=["a", "b"], + output_json=False, + ) + + self.assertEqual(ctx.exception.code, 1) + mock_error.assert_called_once() + self.assertIn("only valid for string", mock_error.call_args[0][0]) + + @patch("poly.cli_commands.metrics.json_print") + @patch("poly.cli_commands.metrics.load_project") + def test_add_expected_values_rejected_json(self, mock_load, mock_json): + """In JSON mode, expected values are rejected for non-string types.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + + with self.assertRaises(SystemExit): + MetricsCommand.metrics_add( + "/fake/path", + name="SCORE", + metric_type="int", + expected_values=["a", "b"], + output_json=True, + ) + + printed = mock_json.call_args[0][0] + self.assertFalse(printed["success"]) + self.assertIn("only valid for string", printed["error"]) + @patch("poly.cli_commands.metrics.json_print") @patch("poly.cli_commands.metrics.AgentStudioInterface.create_custom_metric") @patch("poly.cli_commands.metrics.load_project") @@ -360,6 +402,26 @@ def test_edit_no_flags_json_exits(self, mock_load, mock_json): printed = mock_json.call_args[0][0] self.assertFalse(printed["success"]) + @patch("poly.cli_commands.metrics.error") + @patch("poly.cli_commands.metrics.AgentStudioInterface.get_custom_metrics") + @patch("poly.cli_commands.metrics.load_project") + def test_edit_expected_values_rejected_for_non_string(self, mock_load, mock_get, mock_error): + """Expected values flag is rejected when the metric type is not string.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + mock_get.return_value = [{"name": "SCORE", "type": "int"}] + + with self.assertRaises(SystemExit) as ctx: + MetricsCommand.metrics_edit( + "/fake/path", + name="SCORE", + expected_values=["a", "b"], + output_json=False, + ) + + self.assertEqual(ctx.exception.code, 1) + self.assertIn("only valid for string", mock_error.call_args[0][0]) + @patch("poly.cli_commands.metrics.json_print") @patch("poly.cli_commands.metrics.AgentStudioInterface.update_custom_metric") @patch("poly.cli_commands.metrics.load_project") @@ -387,6 +449,14 @@ class InteractiveEditTest(unittest.TestCase): "active": True, "expected_values": [], }, + { + "name": "STATUS", + "type": "string", + "description": "Call status", + "api": False, + "active": True, + "expected_values": ["open", "closed"], + }, ] @patch("poly.cli_commands.metrics.AgentStudioInterface.get_custom_metrics") @@ -459,17 +529,31 @@ def test_edit_api_and_active(self, mock_get, mock_checkbox, mock_confirm): @patch("questionary.text") @patch("questionary.checkbox") @patch("poly.cli_commands.metrics.AgentStudioInterface.get_custom_metrics") - def test_edit_expected_values(self, mock_get, mock_checkbox, mock_text): - """Parses space-separated expected values.""" + def test_edit_expected_values_string_metric(self, mock_get, mock_checkbox, mock_text): + """Parses space-separated expected values for string metrics.""" project = MagicMock(region="us", account_id="acc1", project_id="proj1") mock_get.return_value = self.SAMPLE_METRICS mock_checkbox.return_value.ask.return_value = ["expected_values"] mock_text.return_value.ask.return_value = "low medium high" - result = MetricsCommand._interactive_edit(project, "SCORE") + result = MetricsCommand._interactive_edit(project, "STATUS") self.assertEqual(result, {"expected_values": ["low", "medium", "high"]}) + @patch("questionary.checkbox") + @patch("poly.cli_commands.metrics.AgentStudioInterface.get_custom_metrics") + def test_no_expected_values_for_non_string(self, mock_get, mock_checkbox): + """expected_values is not offered as a choice for non-string metrics.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_get.return_value = self.SAMPLE_METRICS + mock_checkbox.return_value.ask.return_value = [] + + with self.assertRaises(SystemExit): + MetricsCommand._interactive_edit(project, "SCORE") + + choices = mock_checkbox.call_args[1]["choices"] + self.assertNotIn("expected_values", choices) + class MetricsImportTest(unittest.TestCase): """Tests for MetricsCommand.metrics_import.""" From 0b99540672d086980f694db39593ffa4d42cd880 Mon Sep 17 00:00:00 2001 From: bill-parker Date: Fri, 7 Aug 2026 14:00:46 +0100 Subject: [PATCH 11/15] test: add tests for friendly errors and dict import response handling Co-Authored-By: Claude Opus 4.6 --- src/poly/tests/metrics_test.py | 108 +++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/src/poly/tests/metrics_test.py b/src/poly/tests/metrics_test.py index 8edda6c0..c45d11aa 100644 --- a/src/poly/tests/metrics_test.py +++ b/src/poly/tests/metrics_test.py @@ -315,6 +315,48 @@ def test_add_json_output_on_success(self, mock_load, mock_create, mock_json): mock_json.assert_called_once_with({"success": True, "metric": result}) + @patch("poly.cli_commands.metrics.error") + @patch("poly.cli_commands.metrics.AgentStudioInterface.create_custom_metric") + @patch("poly.cli_commands.metrics.load_project") + def test_add_duplicate_metric_friendly_error(self, mock_load, mock_create, mock_error): + """409 from the server shows 'already exists' instead of raw HTTP error.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + response = MagicMock(status_code=409, text="conflict") + mock_create.side_effect = __import__("requests").HTTPError(response=response) + + with self.assertRaises(SystemExit) as ctx: + MetricsCommand.metrics_add( + "/fake/path", + name="SCORE", + metric_type="int", + description="desc", + api=True, + output_json=False, + ) + + self.assertEqual(ctx.exception.code, 1) + self.assertIn("already exists", mock_error.call_args[0][0]) + + @patch("poly.cli_commands.metrics.json_print") + @patch("poly.cli_commands.metrics.AgentStudioInterface.create_custom_metric") + @patch("poly.cli_commands.metrics.load_project") + def test_add_duplicate_metric_json(self, mock_load, mock_create, mock_json): + """409 in JSON mode prints structured error with 'already exists'.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + response = MagicMock(status_code=409, text="conflict") + mock_create.side_effect = __import__("requests").HTTPError(response=response) + + with self.assertRaises(SystemExit): + MetricsCommand.metrics_add( + "/fake/path", name="SCORE", metric_type="int", output_json=True + ) + + printed = mock_json.call_args[0][0] + self.assertFalse(printed["success"]) + self.assertIn("already exists", printed["error"]) + class MetricsEditTest(unittest.TestCase): """Tests for MetricsCommand.metrics_edit.""" @@ -402,6 +444,43 @@ def test_edit_no_flags_json_exits(self, mock_load, mock_json): printed = mock_json.call_args[0][0] self.assertFalse(printed["success"]) + @patch("poly.cli_commands.metrics.error") + @patch("poly.cli_commands.metrics.AgentStudioInterface.update_custom_metric") + @patch("poly.cli_commands.metrics.load_project") + def test_edit_not_found_friendly_error(self, mock_load, mock_update, mock_error): + """404 from the server shows 'not found' instead of raw HTTP error.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + response = MagicMock(status_code=404, text="not found") + mock_update.side_effect = __import__("requests").HTTPError(response=response) + + with self.assertRaises(SystemExit) as ctx: + MetricsCommand.metrics_edit( + "/fake/path", name="GHOST", description="x", output_json=False + ) + + self.assertEqual(ctx.exception.code, 1) + self.assertIn("not found", mock_error.call_args[0][0]) + + @patch("poly.cli_commands.metrics.json_print") + @patch("poly.cli_commands.metrics.AgentStudioInterface.update_custom_metric") + @patch("poly.cli_commands.metrics.load_project") + def test_edit_not_found_json(self, mock_load, mock_update, mock_json): + """404 in JSON mode prints structured error with 'not found'.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + response = MagicMock(status_code=404, text="not found") + mock_update.side_effect = __import__("requests").HTTPError(response=response) + + with self.assertRaises(SystemExit): + MetricsCommand.metrics_edit( + "/fake/path", name="GHOST", description="x", output_json=True + ) + + printed = mock_json.call_args[0][0] + self.assertFalse(printed["success"]) + self.assertIn("not found", printed["error"]) + @patch("poly.cli_commands.metrics.error") @patch("poly.cli_commands.metrics.AgentStudioInterface.get_custom_metrics") @patch("poly.cli_commands.metrics.load_project") @@ -624,6 +703,35 @@ def test_import_success(self, mock_load, mock_preview, mock_import, mock_exists) # Verify dry_run=False was passed self.assertFalse(mock_import.call_args[1]["dry_run"]) + @patch("builtins.open", unittest.mock.mock_open(read_data="SCORE:\n type: int\n")) + @patch("os.path.exists", return_value=True) + @patch("poly.cli_commands.metrics.AgentStudioInterface.import_custom_metrics") + @patch("poly.cli_commands.metrics.AgentStudioInterface.preview_metrics_import") + @patch("poly.cli_commands.metrics.load_project") + def test_import_handles_dict_response_items(self, mock_load, mock_preview, mock_import, _): + """Import correctly extracts names from dict-format metadata items.""" + project = MagicMock(region="us", account_id="acc1", project_id="proj1") + mock_load.return_value = project + mock_preview.return_value = {"remote_only": []} + mock_import.return_value = { + "metadata": { + "created": [{"name": "SCORE", "message": "created"}], + "ignored": [{"name": "STATUS", "message": "already exists"}], + }, + } + + with ( + patch("poly.cli_commands.metrics.success") as mock_success, + patch("poly.cli_commands.metrics.plain") as mock_plain, + ): + MetricsCommand.metrics_import("/fake/path", file_path="m.yaml", output_json=False) + + plain_calls = [c[0][0] for c in mock_plain.call_args_list] + self.assertTrue(any("SCORE" in c for c in plain_calls)) + self.assertTrue(any("STATUS" in c for c in plain_calls)) + mock_success.assert_called_once() + self.assertIn("1 metrics", mock_success.call_args[0][0]) + class PrintDryRunTest(unittest.TestCase): """Tests for MetricsCommand._print_dry_run static method.""" From b75f626033c309a88f5daa97b769a730cbbd45d0 Mon Sep 17 00:00:00 2001 From: bill-parker Date: Fri, 7 Aug 2026 14:40:17 +0100 Subject: [PATCH 12/15] chore: rename test path names --- src/poly/tests/metrics_test.py | 60 +++++++++++++++++----------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/src/poly/tests/metrics_test.py b/src/poly/tests/metrics_test.py index c45d11aa..1ac583bc 100644 --- a/src/poly/tests/metrics_test.py +++ b/src/poly/tests/metrics_test.py @@ -46,7 +46,7 @@ def test_list_calls_print_metrics(self, mock_load, mock_get, mock_print): mock_load.return_value = project mock_get.return_value = [{"name": "SCORE", "type": "int"}] - MetricsCommand.metrics_list("/fake/path", output_json=False) + MetricsCommand.metrics_list("/tmp/test", output_json=False) mock_get.assert_called_once_with("us", "acc1", "proj1") mock_print.assert_called_once_with([{"name": "SCORE", "type": "int"}]) @@ -61,7 +61,7 @@ def test_list_json_output(self, mock_load, mock_get, mock_json): metrics = [{"name": "SCORE"}] mock_get.return_value = metrics - MetricsCommand.metrics_list("/fake/path", output_json=True) + MetricsCommand.metrics_list("/tmp/test", output_json=True) mock_json.assert_called_once_with(metrics) @@ -79,7 +79,7 @@ def test_export_json_output(self, mock_load, mock_export, mock_json): data = {"SCORE": {"type": "int"}, "STATUS": {"type": "string"}} mock_export.return_value = data - MetricsCommand.metrics_export("/fake/path", output_json=True) + MetricsCommand.metrics_export("/tmp/test", output_json=True) mock_json.assert_called_once_with(data) @@ -94,7 +94,7 @@ def test_export_to_stdout(self, mock_load, mock_export): with patch("poly.cli_commands.metrics.YAML") as mock_yaml_cls: mock_ry = MagicMock() mock_yaml_cls.return_value = mock_ry - MetricsCommand.metrics_export("/fake/path", file_path=None, output_json=False) + MetricsCommand.metrics_export("/tmp/test", file_path=None, output_json=False) import sys @@ -115,7 +115,7 @@ def test_export_to_file(self, mock_load, mock_export, mock_success): ): mock_ry = MagicMock() mock_yaml_cls.return_value = mock_ry - MetricsCommand.metrics_export("/fake/path", file_path="out.yaml", output_json=False) + MetricsCommand.metrics_export("/tmp/test", file_path="out.yaml", output_json=False) mock_file.assert_called_once_with("out.yaml", "w") mock_ry.dump.assert_called_once() @@ -133,7 +133,7 @@ def test_export_empty_metrics(self, mock_load, mock_export): with patch("poly.cli_commands.metrics.YAML") as mock_yaml_cls: mock_ry = MagicMock() mock_yaml_cls.return_value = mock_ry - MetricsCommand.metrics_export("/fake/path", output_json=False) + MetricsCommand.metrics_export("/tmp/test", output_json=False) mock_ry.dump.assert_called_once_with({}, unittest.mock.ANY) @@ -153,7 +153,7 @@ def test_add_with_all_args(self, mock_load, mock_create, mock_update, mock_succe mock_update.return_value = {"name": "SCORE", "type": "int", "api": True} MetricsCommand.metrics_add( - "/fake/path", + "/tmp/test", name="SCORE", metric_type="int", description="CSAT Score", @@ -182,7 +182,7 @@ def test_add_without_api_skips_update(self, mock_load, mock_create, mock_update, mock_create.return_value = {"name": "SCORE", "type": "int"} MetricsCommand.metrics_add( - "/fake/path", + "/tmp/test", name="SCORE", metric_type="int", api=False, @@ -202,7 +202,7 @@ def test_add_passes_expected_values(self, mock_load, mock_create, mock_json): mock_create.return_value = {} MetricsCommand.metrics_add( - "/fake/path", + "/tmp/test", name="STATUS", metric_type="string", description=None, @@ -223,7 +223,7 @@ def test_add_json_error_when_name_missing(self, mock_load, mock_json): with self.assertRaises(SystemExit) as ctx: MetricsCommand.metrics_add( - "/fake/path", + "/tmp/test", name=None, metric_type="int", output_json=True, @@ -244,7 +244,7 @@ def test_add_json_error_when_type_missing(self, mock_load, mock_json): with self.assertRaises(SystemExit) as ctx: MetricsCommand.metrics_add( - "/fake/path", + "/tmp/test", name="SCORE", metric_type=None, output_json=True, @@ -263,7 +263,7 @@ def test_add_expected_values_rejected_for_non_string(self, mock_load, mock_error with self.assertRaises(SystemExit) as ctx: MetricsCommand.metrics_add( - "/fake/path", + "/tmp/test", name="SCORE", metric_type="int", description="desc", @@ -285,7 +285,7 @@ def test_add_expected_values_rejected_json(self, mock_load, mock_json): with self.assertRaises(SystemExit): MetricsCommand.metrics_add( - "/fake/path", + "/tmp/test", name="SCORE", metric_type="int", expected_values=["a", "b"], @@ -307,7 +307,7 @@ def test_add_json_output_on_success(self, mock_load, mock_create, mock_json): mock_create.return_value = result MetricsCommand.metrics_add( - "/fake/path", + "/tmp/test", name="SCORE", metric_type="int", output_json=True, @@ -327,7 +327,7 @@ def test_add_duplicate_metric_friendly_error(self, mock_load, mock_create, mock_ with self.assertRaises(SystemExit) as ctx: MetricsCommand.metrics_add( - "/fake/path", + "/tmp/test", name="SCORE", metric_type="int", description="desc", @@ -350,7 +350,7 @@ def test_add_duplicate_metric_json(self, mock_load, mock_create, mock_json): with self.assertRaises(SystemExit): MetricsCommand.metrics_add( - "/fake/path", name="SCORE", metric_type="int", output_json=True + "/tmp/test", name="SCORE", metric_type="int", output_json=True ) printed = mock_json.call_args[0][0] @@ -371,7 +371,7 @@ def test_edit_with_description(self, mock_load, mock_update, mock_success): mock_update.return_value = {} MetricsCommand.metrics_edit( - "/fake/path", name="SCORE", description="New desc", output_json=False + "/tmp/test", name="SCORE", description="New desc", output_json=False ) mock_update.assert_called_once_with( @@ -387,7 +387,7 @@ def test_edit_deactivate_metric(self, mock_load, mock_update, mock_success): mock_load.return_value = project mock_update.return_value = {} - MetricsCommand.metrics_edit("/fake/path", name="SCORE", active=False, output_json=False) + MetricsCommand.metrics_edit("/tmp/test", name="SCORE", active=False, output_json=False) mock_update.assert_called_once_with("us", "acc1", "proj1", "SCORE", {"active": False}) mock_success.assert_called_once() @@ -403,7 +403,7 @@ def test_edit_multiple_flags(self, mock_load, mock_update, mock_success): mock_update.return_value = {} MetricsCommand.metrics_edit( - "/fake/path", + "/tmp/test", name="SCORE", description="Updated", api=True, @@ -426,7 +426,7 @@ def test_edit_no_flags_triggers_interactive(self, mock_load, mock_update, mock_i mock_interactive.return_value = {"description": "New desc"} mock_update.return_value = {"name": "SCORE", "description": "New desc"} - MetricsCommand.metrics_edit("/fake/path", name="SCORE", output_json=False) + MetricsCommand.metrics_edit("/tmp/test", name="SCORE", output_json=False) mock_interactive.assert_called_once_with(project, "SCORE") mock_update.assert_called_once() @@ -439,7 +439,7 @@ def test_edit_no_flags_json_exits(self, mock_load, mock_json): mock_load.return_value = project with self.assertRaises(SystemExit): - MetricsCommand.metrics_edit("/fake/path", name="SCORE", output_json=True) + MetricsCommand.metrics_edit("/tmp/test", name="SCORE", output_json=True) printed = mock_json.call_args[0][0] self.assertFalse(printed["success"]) @@ -456,7 +456,7 @@ def test_edit_not_found_friendly_error(self, mock_load, mock_update, mock_error) with self.assertRaises(SystemExit) as ctx: MetricsCommand.metrics_edit( - "/fake/path", name="GHOST", description="x", output_json=False + "/tmp/test", name="GHOST", description="x", output_json=False ) self.assertEqual(ctx.exception.code, 1) @@ -474,7 +474,7 @@ def test_edit_not_found_json(self, mock_load, mock_update, mock_json): with self.assertRaises(SystemExit): MetricsCommand.metrics_edit( - "/fake/path", name="GHOST", description="x", output_json=True + "/tmp/test", name="GHOST", description="x", output_json=True ) printed = mock_json.call_args[0][0] @@ -492,7 +492,7 @@ def test_edit_expected_values_rejected_for_non_string(self, mock_load, mock_get, with self.assertRaises(SystemExit) as ctx: MetricsCommand.metrics_edit( - "/fake/path", + "/tmp/test", name="SCORE", expected_values=["a", "b"], output_json=False, @@ -511,7 +511,7 @@ def test_edit_json_output(self, mock_load, mock_update, mock_json): result = {"name": "SCORE", "active": True} mock_update.return_value = result - MetricsCommand.metrics_edit("/fake/path", name="SCORE", active=True, output_json=True) + MetricsCommand.metrics_edit("/tmp/test", name="SCORE", active=True, output_json=True) mock_json.assert_called_once_with({"success": True, "metric": result}) @@ -646,7 +646,7 @@ def test_import_file_not_found(self, mock_load, mock_error): with self.assertRaises(SystemExit) as ctx: MetricsCommand.metrics_import( - "/fake/path", file_path="/nonexistent/metrics.yaml", output_json=False + "/tmp/test", file_path="/nonexistent/metrics.yaml", output_json=False ) self.assertEqual(ctx.exception.code, 1) @@ -662,7 +662,7 @@ def test_import_file_not_found_json(self, mock_load, mock_json): with self.assertRaises(SystemExit): MetricsCommand.metrics_import( - "/fake/path", file_path="/nonexistent/metrics.yaml", output_json=True + "/tmp/test", file_path="/nonexistent/metrics.yaml", output_json=True ) printed = mock_json.call_args[0][0] @@ -678,7 +678,7 @@ def test_import_invalid_yaml(self, mock_load, mock_error, mock_exists): mock_load.return_value = project with self.assertRaises(SystemExit) as ctx: - MetricsCommand.metrics_import("/fake/path", file_path="bad.yaml", output_json=False) + MetricsCommand.metrics_import("/tmp/test", file_path="bad.yaml", output_json=False) self.assertEqual(ctx.exception.code, 1) @@ -697,7 +697,7 @@ def test_import_success(self, mock_load, mock_preview, mock_import, mock_exists) } with patch("poly.cli_commands.metrics.success"), patch("poly.cli_commands.metrics.plain"): - MetricsCommand.metrics_import("/fake/path", file_path="metrics.yaml", output_json=False) + MetricsCommand.metrics_import("/tmp/test", file_path="metrics.yaml", output_json=False) mock_import.assert_called_once() # Verify dry_run=False was passed @@ -724,7 +724,7 @@ def test_import_handles_dict_response_items(self, mock_load, mock_preview, mock_ patch("poly.cli_commands.metrics.success") as mock_success, patch("poly.cli_commands.metrics.plain") as mock_plain, ): - MetricsCommand.metrics_import("/fake/path", file_path="m.yaml", output_json=False) + MetricsCommand.metrics_import("/tmp/test", file_path="m.yaml", output_json=False) plain_calls = [c[0][0] for c in mock_plain.call_args_list] self.assertTrue(any("SCORE" in c for c in plain_calls)) From afd4dbc3c55a143fef9fbae21575bbabb034259e Mon Sep 17 00:00:00 2001 From: bill-parker Date: Fri, 7 Aug 2026 14:42:58 +0100 Subject: [PATCH 13/15] test: add coverage for platform API, interface, and console metrics code Co-Authored-By: Claude Opus 4.6 --- src/poly/tests/api/platform_api_test.py | 94 +++++++++++++++++++++++++ src/poly/tests/metrics_test.py | 52 ++++++++++++++ 2 files changed, 146 insertions(+) diff --git a/src/poly/tests/api/platform_api_test.py b/src/poly/tests/api/platform_api_test.py index e233ad10..c465bc61 100644 --- a/src/poly/tests/api/platform_api_test.py +++ b/src/poly/tests/api/platform_api_test.py @@ -459,5 +459,99 @@ def test_error_status_raises_http_error(self, mock_request, _mock_key): PlatformAPIHandler.synthesize_audio_cache("studio", "agent-1", "entry-1", "hi", {}) +class GetCustomMetrics(unittest.TestCase): + """Tests for PlatformAPIHandler.get_custom_metrics.""" + + @patch("poly.handlers.platform_api.retrieve_api_key", return_value="secret-key") + @patch("poly.handlers.platform_api.requests.request") + def test_returns_list_directly(self, mock_request, _mock_key): + """When the API returns a bare list, it is returned as-is.""" + metrics = [{"name": "SCORE", "type": "int"}] + mock_request.return_value = make_mock_response(200, json_body=metrics) + + result = PlatformAPIHandler.get_custom_metrics("studio", "acc1", "proj1") + + self.assertEqual(result, metrics) + + @patch("poly.handlers.platform_api.retrieve_api_key", return_value="secret-key") + @patch("poly.handlers.platform_api.requests.request") + def test_extracts_from_metrics_key(self, mock_request, _mock_key): + """When the API wraps the list in a 'metrics' key, it is unwrapped.""" + metrics = [{"name": "SCORE", "type": "int"}] + mock_request.return_value = make_mock_response(200, json_body={"metrics": metrics}) + + result = PlatformAPIHandler.get_custom_metrics("studio", "acc1", "proj1") + + self.assertEqual(result, metrics) + + +class CreateCustomMetric(unittest.TestCase): + """Tests for PlatformAPIHandler.create_custom_metric.""" + + @patch("poly.handlers.platform_api.retrieve_api_key", return_value="secret-key") + @patch("poly.handlers.platform_api.requests.request") + def test_posts_data_to_correct_endpoint(self, mock_request, _mock_key): + """create_custom_metric sends a POST with the metric payload.""" + mock_request.return_value = make_mock_response(200, json_body={"name": "SCORE"}) + + data = {"name": "SCORE", "type": "int"} + PlatformAPIHandler.create_custom_metric("studio", "acc1", "proj1", data) + + call_kwargs = mock_request.call_args + self.assertEqual(call_kwargs.kwargs["method"], "POST") + self.assertIn("/custom-metrics", call_kwargs.kwargs["url"]) + + +class UpdateCustomMetric(unittest.TestCase): + """Tests for PlatformAPIHandler.update_custom_metric.""" + + @patch("poly.handlers.platform_api.retrieve_api_key", return_value="secret-key") + @patch("poly.handlers.platform_api.requests.request") + def test_patches_correct_metric(self, mock_request, _mock_key): + """update_custom_metric sends a PATCH to the metric-specific URL.""" + mock_request.return_value = make_mock_response(200, json_body={"name": "SCORE"}) + + PlatformAPIHandler.update_custom_metric("studio", "acc1", "proj1", "SCORE", {"api": True}) + + call_kwargs = mock_request.call_args + self.assertEqual(call_kwargs.kwargs["method"], "PATCH") + self.assertIn("/custom-metrics/SCORE", call_kwargs.kwargs["url"]) + + +class ExportCustomMetrics(unittest.TestCase): + """Tests for PlatformAPIHandler.export_custom_metrics.""" + + @patch("poly.handlers.platform_api.retrieve_api_key", return_value="secret-key") + @patch("poly.handlers.platform_api.requests.request") + def test_requests_yaml_format(self, mock_request, _mock_key): + """export_custom_metrics hits the export endpoint with a GET.""" + yaml_body = b"SCORE:\n type: int\n" + mock_request.return_value = make_mock_response(200, content=yaml_body) + + PlatformAPIHandler.export_custom_metrics("studio", "acc1", "proj1") + + call_kwargs = mock_request.call_args + self.assertEqual(call_kwargs.kwargs["method"], "GET") + self.assertIn("/export", call_kwargs.kwargs["url"]) + + +class ImportCustomMetrics(unittest.TestCase): + """Tests for PlatformAPIHandler.import_custom_metrics.""" + + @patch("poly.handlers.platform_api.retrieve_api_key", return_value="secret-key") + @patch("poly.handlers.platform_api.requests.request") + def test_sends_multipart_upload(self, mock_request, _mock_key): + """import_custom_metrics POSTs a multipart file upload.""" + mock_request.return_value = make_mock_response( + 200, json_body={"metadata": {"created": [], "ignored": []}} + ) + + PlatformAPIHandler.import_custom_metrics("studio", "acc1", "proj1", "SCORE:\n type: int\n") + + call_kwargs = mock_request.call_args + self.assertIn("/import", call_kwargs.kwargs["url"]) + self.assertIn("yaml", call_kwargs.kwargs.get("files", {})) + + if __name__ == "__main__": unittest.main() diff --git a/src/poly/tests/metrics_test.py b/src/poly/tests/metrics_test.py index 1ac583bc..dc70a48b 100644 --- a/src/poly/tests/metrics_test.py +++ b/src/poly/tests/metrics_test.py @@ -781,6 +781,58 @@ def test_dry_run_empty_preview(self, mock_plain): self.assertEqual(mock_plain.call_count, 1) +class PrintMetricsTest(unittest.TestCase): + """Tests for print_metrics console output.""" + + @patch("poly.output.console.plain") + def test_empty_metrics(self, mock_plain): + """Prints 'No metrics found.' when list is empty.""" + from poly.output.console import print_metrics + + print_metrics([]) + + mock_plain.assert_called_once_with("No metrics found.") + + @patch("poly.output.console.console") + def test_renders_table_with_counts(self, mock_console): + """Renders a table and prints active/inactive summary.""" + from poly.output.console import print_metrics + + metrics = [ + {"name": "SCORE", "type": "int", "active": True, "api": False}, + {"name": "OLD", "type": "string", "active": False, "api": True, "description": "old"}, + ] + + print_metrics(metrics) + + self.assertEqual(mock_console.print.call_count, 2) + summary = mock_console.print.call_args_list[1][0][0] + self.assertIn("1 active", summary) + self.assertIn("1 inactive", summary) + + +class PreviewMetricsImportTest(unittest.TestCase): + """Tests for AgentStudioInterface.preview_metrics_import.""" + + @patch("poly.handlers.interface.PlatformAPIHandler.get_custom_metrics") + def test_computes_set_diff(self, mock_get): + """Correctly partitions local and remote metrics.""" + from poly.handlers.interface import AgentStudioInterface + + mock_get.return_value = [ + {"name": "EXISTING"}, + {"name": "REMOTE_ONLY"}, + ] + + result = AgentStudioInterface.preview_metrics_import( + "us", "acc1", "proj1", {"EXISTING", "NEW_ONE"} + ) + + self.assertEqual(result["would_create"], ["NEW_ONE"]) + self.assertEqual(result["would_skip"], ["EXISTING"]) + self.assertEqual(result["remote_only"], ["REMOTE_ONLY"]) + + class ValidMetricTypesTest(unittest.TestCase): """Tests for the VALID_METRIC_TYPES constant.""" From 46e3c9a87836e1dc4c3901f46a97896a952594ee Mon Sep 17 00:00:00 2001 From: bill-parker Date: Fri, 7 Aug 2026 15:11:49 +0100 Subject: [PATCH 14/15] refactor: move metrics business logic from CLI to interface layer --- src/poly/cli_commands/metrics.py | 94 +++++-------------- src/poly/handlers/interface.py | 117 ++++++++++++++++++++---- src/poly/handlers/platform_api.py | 28 ++++++ src/poly/tests/api/platform_api_test.py | 17 ++++ src/poly/tests/metrics_test.py | 75 ++++++++------- 5 files changed, 206 insertions(+), 125 deletions(-) diff --git a/src/poly/cli_commands/metrics.py b/src/poly/cli_commands/metrics.py index 997cf284..e46befad 100644 --- a/src/poly/cli_commands/metrics.py +++ b/src/poly/cli_commands/metrics.py @@ -4,12 +4,11 @@ """ import logging -import os import sys from argparse import ArgumentParser, Namespace, RawTextHelpFormatter, _SubParsersAction import requests -from ruamel.yaml import YAML, YAMLError +from ruamel.yaml import YAML from poly.cli_commands.base import BaseCommand, Parents from poly.cli_commands.shared import load_project @@ -329,14 +328,6 @@ def metrics_add( sys.exit(1) api = api_result - if expected_values and metric_type != "string": - msg = "--expected-values is only valid for string metrics." - if output_json: - json_print({"success": False, "error": msg}) - else: - error(msg) - sys.exit(1) - data: dict = {"name": name, "type": metric_type} if description: data["description"] = description @@ -349,12 +340,12 @@ def metrics_add( result = AgentStudioInterface.create_custom_metric( project.region, project.account_id, project.project_id, data ) - - # The server ignores the api flag on create, so follow up with an edit - if api: - result = AgentStudioInterface.update_custom_metric( - project.region, project.account_id, project.project_id, name, {"api": True} - ) + except ValueError as e: + if output_json: + json_print({"success": False, "error": str(e)}) + else: + error(str(e)) + sys.exit(1) except requests.HTTPError as e: if e.response is not None and e.response.status_code == 409: msg = f"Metric '{name}' already exists." @@ -385,19 +376,6 @@ def metrics_edit( """Update an existing custom metric.""" project = load_project(base_path, output_json=output_json) - if expected_values is not None: - metrics = AgentStudioInterface.get_custom_metrics( - project.region, project.account_id, project.project_id - ) - metric = next((m for m in metrics if m.get("name") == name), None) - if metric and metric.get("type") != "string": - msg = "--expected-values is only valid for string metrics." - if output_json: - json_print({"success": False, "error": msg}) - else: - error(msg) - sys.exit(1) - data: dict = {} if description is not None: data["description"] = description @@ -419,6 +397,12 @@ def metrics_edit( result = AgentStudioInterface.update_custom_metric( project.region, project.account_id, project.project_id, name, data ) + except ValueError as e: + if output_json: + json_print({"success": False, "error": str(e)}) + else: + error(str(e)) + sys.exit(1) except requests.HTTPError as e: if e.response is not None and e.response.status_code == 404: msg = f"Metric '{name}' not found." @@ -542,57 +526,29 @@ def metrics_import( """Bulk-import metrics from a YAML file.""" project = load_project(base_path, output_json=output_json) - if not os.path.exists(file_path): - msg = f"File not found: {file_path}" - if output_json: - json_print({"success": False, "error": msg}) - else: - error(msg) - sys.exit(1) - - with open(file_path) as f: - yaml_content = f.read() - try: - ry = YAML() - local_metrics = ry.load(yaml_content) or {} - except YAMLError as e: - msg = f"Invalid YAML: {e}" + result = AgentStudioInterface.import_metrics_from_file( + project.region, project.account_id, project.project_id, file_path, dry_run + ) + except (FileNotFoundError, ValueError) as e: if output_json: - json_print({"success": False, "error": msg}) + json_print({"success": False, "error": str(e)}) else: - error(msg) + error(str(e)) sys.exit(1) - local_names = set(local_metrics.keys()) - - preview = AgentStudioInterface.preview_metrics_import( - project.region, project.account_id, project.project_id, local_names - ) - if dry_run: - cls._print_dry_run(preview, output_json) + cls._print_dry_run(result, output_json) return - # Warn about metrics not in the file - if preview["remote_only"] and not output_json: - warning( - f"Metrics on remote but not in file (not deleted):" - f" {', '.join(preview['remote_only'])}" - ) - - import_result = AgentStudioInterface.import_custom_metrics( - project.region, - project.account_id, - project.project_id, - yaml_content, - dry_run=False, - ) + remote_only = result.get("remote_only", []) + if remote_only and not output_json: + warning(f"Metrics on remote but not in file (not deleted): {', '.join(remote_only)}") if output_json: - json_print({"success": True, **import_result}) + json_print({"success": True, **result}) else: - metadata = import_result.get("metadata", {}) + metadata = result.get("metadata", {}) created = metadata.get("created", []) ignored = metadata.get("ignored", []) diff --git a/src/poly/handlers/interface.py b/src/poly/handlers/interface.py index 6c930252..9eea440e 100644 --- a/src/poly/handlers/interface.py +++ b/src/poly/handlers/interface.py @@ -65,7 +65,7 @@ def _extract_error_code(e: Exception) -> Optional[str]: if response is not None: try: return response.json().get("error_code") - except (json.JSONDecodeError, ValueError, AttributeError): + except json.JSONDecodeError, ValueError, AttributeError: pass return None @@ -1284,6 +1284,10 @@ def create_custom_metric( ) -> dict: """Create a new custom metric. + Validates that ``expected_values`` is only set for string-type metrics. + Works around a server bug where the ``api`` flag is ignored on create + by issuing a follow-up PATCH when ``api`` is ``True``. + Args: region: The region name. account_id: The account ID. @@ -1292,8 +1296,22 @@ def create_custom_metric( Returns: dict: The created metric record. + + Raises: + ValueError: If expected_values is set for a non-string metric. """ - return PlatformAPIHandler.create_custom_metric(region, account_id, project_id, data) + if data.get("expected_values") and data.get("type") != "string": + raise ValueError("--expected-values is only valid for string metrics.") + + result = PlatformAPIHandler.create_custom_metric(region, account_id, project_id, data) + + # The server ignores the api flag on create, so follow up with an edit + if data.get("api"): + result = PlatformAPIHandler.update_custom_metric( + region, account_id, project_id, data["name"], {"api": True} + ) + + return result @staticmethod def update_custom_metric( @@ -1305,6 +1323,10 @@ def update_custom_metric( ) -> dict: """Update an existing custom metric. + Validates that ``expected_values`` is only set for string-type metrics + by fetching the metric's type from the API when ``expected_values`` + is present. + Args: region: The region name. account_id: The account ID. @@ -1314,7 +1336,16 @@ def update_custom_metric( Returns: dict: The updated metric record. + + Raises: + ValueError: If expected_values is set for a non-string metric. """ + if data.get("expected_values") is not None: + metrics = PlatformAPIHandler.get_custom_metrics(region, account_id, project_id) + metric = next((m for m in metrics if m.get("name") == metric_name), None) + if metric and metric.get("type") != "string": + raise ValueError("--expected-values is only valid for string metrics.") + return PlatformAPIHandler.update_custom_metric( region, account_id, project_id, metric_name, data ) @@ -1346,9 +1377,6 @@ def preview_metrics_import( ) -> dict[str, list[str]]: """Fetch remote metrics and compute what an import would do. - Compares the local metric names against the remote set to determine - which metrics would be created, skipped, or exist only on the remote. - Args: region: The region name. account_id: The account ID. @@ -1359,18 +1387,9 @@ def preview_metrics_import( dict with keys ``would_create``, ``would_skip``, and ``remote_only``, each a sorted list of metric names. """ - remote_metrics = PlatformAPIHandler.get_custom_metrics(region, account_id, project_id) - remote_names = {m["name"] for m in remote_metrics if "name" in m} - - would_create = local_metric_names - remote_names - would_skip = local_metric_names & remote_names - remote_only = remote_names - local_metric_names - - return { - "would_create": sorted(would_create), - "would_skip": sorted(would_skip), - "remote_only": sorted(remote_only), - } + return PlatformAPIHandler.preview_metrics_import( + region, account_id, project_id, local_metric_names + ) @staticmethod def import_custom_metrics( @@ -1396,6 +1415,70 @@ def import_custom_metrics( region, account_id, project_id, yaml_content, dry_run ) + @staticmethod + def import_metrics_from_file( + region: str, + account_id: str, + project_id: str, + file_path: str, + dry_run: bool = False, + ) -> dict: + """Read a YAML file and import its metrics, or preview the import. + + Args: + region: The region name. + account_id: The account ID. + project_id: The project ID. + file_path: Path to the YAML file with metric definitions. + dry_run: If True, return a preview without applying changes. + + Returns: + dict: In dry-run mode, a preview dict with ``would_create``, + ``would_skip``, and ``remote_only``. Otherwise, the import result + with ``metadata.created`` and ``metadata.ignored``. + + Raises: + FileNotFoundError: If the file does not exist. + ValueError: If the file contains invalid YAML. + """ + import os + + from ruamel.yaml import YAML, YAMLError + + if not os.path.exists(file_path): + raise FileNotFoundError(f"File not found: {file_path}") + + with open(file_path) as f: + yaml_content = f.read() + + try: + ry = YAML() + local_metrics = ry.load(yaml_content) or {} + except YAMLError as e: + raise ValueError(f"Invalid YAML: {e}") from e + + local_names = set(local_metrics.keys()) + + if dry_run: + return { + "dry_run": True, + **PlatformAPIHandler.preview_metrics_import( + region, account_id, project_id, local_names + ), + } + + preview = PlatformAPIHandler.preview_metrics_import( + region, account_id, project_id, local_names + ) + + result = PlatformAPIHandler.import_custom_metrics( + region, account_id, project_id, yaml_content, dry_run=False + ) + + result["remote_only"] = preview["remote_only"] + + return result + def list_rtc_configs( region: str, project_id: str, diff --git a/src/poly/handlers/platform_api.py b/src/poly/handlers/platform_api.py index 097bf849..99c094c5 100644 --- a/src/poly/handlers/platform_api.py +++ b/src/poly/handlers/platform_api.py @@ -1448,6 +1448,34 @@ def import_custom_metrics( } return PlatformAPIHandler.make_request(region, endpoint, "POST", params=params, files=files) + @staticmethod + def preview_metrics_import( + region: str, + account_id: str, + project_id: str, + local_metric_names: set[str], + ) -> dict[str, list[str]]: + """Compare local metric names against remote to preview an import. + + Args: + region: The region name. + account_id: The account ID. + project_id: The project ID. + local_metric_names: Set of metric names from the local YAML file. + + Returns: + dict with keys ``would_create``, ``would_skip``, and ``remote_only``, + each a sorted list of metric names. + """ + remote_metrics = PlatformAPIHandler.get_custom_metrics(region, account_id, project_id) + remote_names = {m["name"] for m in remote_metrics if "name" in m} + + return { + "would_create": sorted(local_metric_names - remote_names), + "would_skip": sorted(local_metric_names & remote_names), + "remote_only": sorted(remote_names - local_metric_names), + } + def list_rtc_configs( region: str, project_id: str, diff --git a/src/poly/tests/api/platform_api_test.py b/src/poly/tests/api/platform_api_test.py index c465bc61..3890d938 100644 --- a/src/poly/tests/api/platform_api_test.py +++ b/src/poly/tests/api/platform_api_test.py @@ -553,5 +553,22 @@ def test_sends_multipart_upload(self, mock_request, _mock_key): self.assertIn("yaml", call_kwargs.kwargs.get("files", {})) +class PreviewMetricsImport(unittest.TestCase): + """Tests for PlatformAPIHandler.preview_metrics_import.""" + + @patch.object(PlatformAPIHandler, "get_custom_metrics") + def test_computes_set_diff(self, mock_get): + """Correctly partitions local vs remote metric names.""" + mock_get.return_value = [{"name": "EXISTING"}, {"name": "REMOTE_ONLY"}] + + result = PlatformAPIHandler.preview_metrics_import( + "studio", "acc1", "proj1", {"EXISTING", "NEW_ONE"} + ) + + self.assertEqual(result["would_create"], ["NEW_ONE"]) + self.assertEqual(result["would_skip"], ["EXISTING"]) + self.assertEqual(result["remote_only"], ["REMOTE_ONLY"]) + + if __name__ == "__main__": unittest.main() diff --git a/src/poly/tests/metrics_test.py b/src/poly/tests/metrics_test.py index dc70a48b..ecc2675c 100644 --- a/src/poly/tests/metrics_test.py +++ b/src/poly/tests/metrics_test.py @@ -142,15 +142,13 @@ class MetricsAddTest(unittest.TestCase): """Tests for MetricsCommand.metrics_add.""" @patch("poly.cli_commands.metrics.success") - @patch("poly.cli_commands.metrics.AgentStudioInterface.update_custom_metric") @patch("poly.cli_commands.metrics.AgentStudioInterface.create_custom_metric") @patch("poly.cli_commands.metrics.load_project") - def test_add_with_all_args(self, mock_load, mock_create, mock_update, mock_success): + def test_add_with_all_args(self, mock_load, mock_create, mock_success): """Non-interactive add passes all fields to create_custom_metric.""" project = MagicMock(region="us", account_id="acc1", project_id="proj1") mock_load.return_value = project - mock_create.return_value = {"name": "SCORE", "type": "int"} - mock_update.return_value = {"name": "SCORE", "type": "int", "api": True} + mock_create.return_value = {"name": "SCORE", "type": "int", "api": True} MetricsCommand.metrics_add( "/tmp/test", @@ -168,15 +166,13 @@ def test_add_with_all_args(self, mock_load, mock_create, mock_update, mock_succe "proj1", {"name": "SCORE", "type": "int", "description": "CSAT Score", "api": True}, ) - mock_update.assert_called_once_with("us", "acc1", "proj1", "SCORE", {"api": True}) mock_success.assert_called_once() @patch("poly.cli_commands.metrics.json_print") - @patch("poly.cli_commands.metrics.AgentStudioInterface.update_custom_metric") @patch("poly.cli_commands.metrics.AgentStudioInterface.create_custom_metric") @patch("poly.cli_commands.metrics.load_project") - def test_add_without_api_skips_update(self, mock_load, mock_create, mock_update, mock_json): - """When api=False, no follow-up update call is made.""" + def test_add_without_api(self, mock_load, mock_create, mock_json): + """When api=False, create is called without the api flag.""" project = MagicMock(region="us", account_id="acc1", project_id="proj1") mock_load.return_value = project mock_create.return_value = {"name": "SCORE", "type": "int"} @@ -190,7 +186,8 @@ def test_add_without_api_skips_update(self, mock_load, mock_create, mock_update, ) mock_create.assert_called_once() - mock_update.assert_not_called() + data = mock_create.call_args[0][3] + self.assertNotIn("api", data) @patch("poly.cli_commands.metrics.json_print") @patch("poly.cli_commands.metrics.AgentStudioInterface.create_custom_metric") @@ -255,11 +252,13 @@ def test_add_json_error_when_type_missing(self, mock_load, mock_json): self.assertIn("--type", printed["error"]) @patch("poly.cli_commands.metrics.error") + @patch("poly.cli_commands.metrics.AgentStudioInterface.create_custom_metric") @patch("poly.cli_commands.metrics.load_project") - def test_add_expected_values_rejected_for_non_string(self, mock_load, mock_error): + def test_add_expected_values_rejected_for_non_string(self, mock_load, mock_create, mock_error): """Expected values are rejected for non-string metric types.""" project = MagicMock(region="us", account_id="acc1", project_id="proj1") mock_load.return_value = project + mock_create.side_effect = ValueError("--expected-values is only valid for string metrics.") with self.assertRaises(SystemExit) as ctx: MetricsCommand.metrics_add( @@ -277,11 +276,13 @@ def test_add_expected_values_rejected_for_non_string(self, mock_load, mock_error self.assertIn("only valid for string", mock_error.call_args[0][0]) @patch("poly.cli_commands.metrics.json_print") + @patch("poly.cli_commands.metrics.AgentStudioInterface.create_custom_metric") @patch("poly.cli_commands.metrics.load_project") - def test_add_expected_values_rejected_json(self, mock_load, mock_json): + def test_add_expected_values_rejected_json(self, mock_load, mock_create, mock_json): """In JSON mode, expected values are rejected for non-string types.""" project = MagicMock(region="us", account_id="acc1", project_id="proj1") mock_load.return_value = project + mock_create.side_effect = ValueError("--expected-values is only valid for string metrics.") with self.assertRaises(SystemExit): MetricsCommand.metrics_add( @@ -482,13 +483,13 @@ def test_edit_not_found_json(self, mock_load, mock_update, mock_json): self.assertIn("not found", printed["error"]) @patch("poly.cli_commands.metrics.error") - @patch("poly.cli_commands.metrics.AgentStudioInterface.get_custom_metrics") + @patch("poly.cli_commands.metrics.AgentStudioInterface.update_custom_metric") @patch("poly.cli_commands.metrics.load_project") - def test_edit_expected_values_rejected_for_non_string(self, mock_load, mock_get, mock_error): + def test_edit_expected_values_rejected_for_non_string(self, mock_load, mock_update, mock_error): """Expected values flag is rejected when the metric type is not string.""" project = MagicMock(region="us", account_id="acc1", project_id="proj1") mock_load.return_value = project - mock_get.return_value = [{"name": "SCORE", "type": "int"}] + mock_update.side_effect = ValueError("--expected-values is only valid for string metrics.") with self.assertRaises(SystemExit) as ctx: MetricsCommand.metrics_edit( @@ -638,11 +639,13 @@ class MetricsImportTest(unittest.TestCase): """Tests for MetricsCommand.metrics_import.""" @patch("poly.cli_commands.metrics.error") + @patch("poly.cli_commands.metrics.AgentStudioInterface.import_metrics_from_file") @patch("poly.cli_commands.metrics.load_project") - def test_import_file_not_found(self, mock_load, mock_error): + def test_import_file_not_found(self, mock_load, mock_import, mock_error): """Exits with error when the import file does not exist.""" project = MagicMock(region="us", account_id="acc1", project_id="proj1") mock_load.return_value = project + mock_import.side_effect = FileNotFoundError("File not found: /nonexistent/metrics.yaml") with self.assertRaises(SystemExit) as ctx: MetricsCommand.metrics_import( @@ -654,11 +657,13 @@ def test_import_file_not_found(self, mock_load, mock_error): self.assertIn("File not found", mock_error.call_args[0][0]) @patch("poly.cli_commands.metrics.json_print") + @patch("poly.cli_commands.metrics.AgentStudioInterface.import_metrics_from_file") @patch("poly.cli_commands.metrics.load_project") - def test_import_file_not_found_json(self, mock_load, mock_json): + def test_import_file_not_found_json(self, mock_load, mock_import, mock_json): """In JSON mode, missing file prints error JSON and exits.""" project = MagicMock(region="us", account_id="acc1", project_id="proj1") mock_load.return_value = project + mock_import.side_effect = FileNotFoundError("File not found: /nonexistent/metrics.yaml") with self.assertRaises(SystemExit): MetricsCommand.metrics_import( @@ -668,52 +673,44 @@ def test_import_file_not_found_json(self, mock_load, mock_json): printed = mock_json.call_args[0][0] self.assertFalse(printed["success"]) - @patch("builtins.open", unittest.mock.mock_open(read_data="{{invalid")) - @patch("os.path.exists", return_value=True) @patch("poly.cli_commands.metrics.error") + @patch("poly.cli_commands.metrics.AgentStudioInterface.import_metrics_from_file") @patch("poly.cli_commands.metrics.load_project") - def test_import_invalid_yaml(self, mock_load, mock_error, mock_exists): + def test_import_invalid_yaml(self, mock_load, mock_import, mock_error): """Exits with error when YAML parsing fails.""" project = MagicMock(region="us", account_id="acc1", project_id="proj1") mock_load.return_value = project + mock_import.side_effect = ValueError("Invalid YAML: ...") with self.assertRaises(SystemExit) as ctx: MetricsCommand.metrics_import("/tmp/test", file_path="bad.yaml", output_json=False) self.assertEqual(ctx.exception.code, 1) - @patch("builtins.open", unittest.mock.mock_open(read_data="SCORE:\n type: int\n")) - @patch("os.path.exists", return_value=True) - @patch("poly.cli_commands.metrics.AgentStudioInterface.import_custom_metrics") - @patch("poly.cli_commands.metrics.AgentStudioInterface.preview_metrics_import") + @patch("poly.cli_commands.metrics.AgentStudioInterface.import_metrics_from_file") @patch("poly.cli_commands.metrics.load_project") - def test_import_success(self, mock_load, mock_preview, mock_import, mock_exists): - """Successful import calls import_custom_metrics and prints summary.""" + def test_import_success(self, mock_load, mock_import): + """Successful import calls import_metrics_from_file and prints summary.""" project = MagicMock(region="us", account_id="acc1", project_id="proj1") mock_load.return_value = project - mock_preview.return_value = {"remote_only": []} mock_import.return_value = { + "remote_only": [], "metadata": {"created": ["SCORE"], "ignored": []}, } with patch("poly.cli_commands.metrics.success"), patch("poly.cli_commands.metrics.plain"): MetricsCommand.metrics_import("/tmp/test", file_path="metrics.yaml", output_json=False) - mock_import.assert_called_once() - # Verify dry_run=False was passed - self.assertFalse(mock_import.call_args[1]["dry_run"]) + mock_import.assert_called_once_with("us", "acc1", "proj1", "metrics.yaml", False) - @patch("builtins.open", unittest.mock.mock_open(read_data="SCORE:\n type: int\n")) - @patch("os.path.exists", return_value=True) - @patch("poly.cli_commands.metrics.AgentStudioInterface.import_custom_metrics") - @patch("poly.cli_commands.metrics.AgentStudioInterface.preview_metrics_import") + @patch("poly.cli_commands.metrics.AgentStudioInterface.import_metrics_from_file") @patch("poly.cli_commands.metrics.load_project") - def test_import_handles_dict_response_items(self, mock_load, mock_preview, mock_import, _): + def test_import_handles_dict_response_items(self, mock_load, mock_import): """Import correctly extracts names from dict-format metadata items.""" project = MagicMock(region="us", account_id="acc1", project_id="proj1") mock_load.return_value = project - mock_preview.return_value = {"remote_only": []} mock_import.return_value = { + "remote_only": [], "metadata": { "created": [{"name": "SCORE", "message": "created"}], "ignored": [{"name": "STATUS", "message": "already exists"}], @@ -812,19 +809,19 @@ def test_renders_table_with_counts(self, mock_console): class PreviewMetricsImportTest(unittest.TestCase): - """Tests for AgentStudioInterface.preview_metrics_import.""" + """Tests for PlatformAPIHandler.preview_metrics_import.""" - @patch("poly.handlers.interface.PlatformAPIHandler.get_custom_metrics") + @patch("poly.handlers.platform_api.PlatformAPIHandler.get_custom_metrics") def test_computes_set_diff(self, mock_get): """Correctly partitions local and remote metrics.""" - from poly.handlers.interface import AgentStudioInterface + from poly.handlers.platform_api import PlatformAPIHandler mock_get.return_value = [ {"name": "EXISTING"}, {"name": "REMOTE_ONLY"}, ] - result = AgentStudioInterface.preview_metrics_import( + result = PlatformAPIHandler.preview_metrics_import( "us", "acc1", "proj1", {"EXISTING", "NEW_ONE"} ) From d866640521eb0b2c91f45b634851f06de6cc631a Mon Sep 17 00:00:00 2001 From: bill-parker Date: Fri, 7 Aug 2026 15:14:51 +0100 Subject: [PATCH 15/15] tests: remove dupes, and change placement of tests --- src/poly/tests/metrics_test.py | 170 ++++++++++++++++++++++++++++++--- 1 file changed, 155 insertions(+), 15 deletions(-) diff --git a/src/poly/tests/metrics_test.py b/src/poly/tests/metrics_test.py index ecc2675c..143fc451 100644 --- a/src/poly/tests/metrics_test.py +++ b/src/poly/tests/metrics_test.py @@ -7,6 +7,7 @@ from unittest.mock import MagicMock, patch from poly.cli_commands.metrics import VALID_METRIC_TYPES, MetricsCommand, _parse_bool_flag +from poly.handlers.interface import AgentStudioInterface class ParseBoolFlagTest(unittest.TestCase): @@ -808,26 +809,165 @@ def test_renders_table_with_counts(self, mock_console): self.assertIn("1 inactive", summary) -class PreviewMetricsImportTest(unittest.TestCase): - """Tests for PlatformAPIHandler.preview_metrics_import.""" +class CreateCustomMetricInterfaceTest(unittest.TestCase): + """Tests for AgentStudioInterface.create_custom_metric business logic.""" - @patch("poly.handlers.platform_api.PlatformAPIHandler.get_custom_metrics") - def test_computes_set_diff(self, mock_get): - """Correctly partitions local and remote metrics.""" - from poly.handlers.platform_api import PlatformAPIHandler + @patch("poly.handlers.interface.PlatformAPIHandler.update_custom_metric") + @patch("poly.handlers.interface.PlatformAPIHandler.create_custom_metric") + def test_api_flag_triggers_follow_up_patch(self, mock_create, mock_update): + """When api=True, a follow-up PATCH sets the api flag after create.""" + mock_create.return_value = {"name": "SCORE", "type": "int"} + mock_update.return_value = {"name": "SCORE", "type": "int", "api": True} - mock_get.return_value = [ - {"name": "EXISTING"}, - {"name": "REMOTE_ONLY"}, - ] + result = AgentStudioInterface.create_custom_metric( + "us", "acc1", "proj1", {"name": "SCORE", "type": "int", "api": True} + ) + + mock_create.assert_called_once() + mock_update.assert_called_once_with("us", "acc1", "proj1", "SCORE", {"api": True}) + self.assertTrue(result["api"]) + + @patch("poly.handlers.interface.PlatformAPIHandler.update_custom_metric") + @patch("poly.handlers.interface.PlatformAPIHandler.create_custom_metric") + def test_no_api_flag_skips_patch(self, mock_create, mock_update): + """When api is not set, no follow-up PATCH is issued.""" + mock_create.return_value = {"name": "SCORE", "type": "int"} + + AgentStudioInterface.create_custom_metric( + "us", "acc1", "proj1", {"name": "SCORE", "type": "int"} + ) + + mock_create.assert_called_once() + mock_update.assert_not_called() + + def test_expected_values_rejected_for_non_string(self): + """Raises ValueError when expected_values is set on a non-string metric.""" + with self.assertRaises(ValueError) as ctx: + AgentStudioInterface.create_custom_metric( + "us", + "acc1", + "proj1", + {"name": "SCORE", "type": "int", "expected_values": ["a", "b"]}, + ) + + self.assertIn("only valid for string", str(ctx.exception)) + + @patch("poly.handlers.interface.PlatformAPIHandler.create_custom_metric") + def test_expected_values_allowed_for_string(self, mock_create): + """Does not raise when expected_values is set on a string metric.""" + mock_create.return_value = {"name": "STATUS", "type": "string"} + + AgentStudioInterface.create_custom_metric( + "us", + "acc1", + "proj1", + {"name": "STATUS", "type": "string", "expected_values": ["open", "closed"]}, + ) + + mock_create.assert_called_once() + + +class UpdateCustomMetricInterfaceTest(unittest.TestCase): + """Tests for AgentStudioInterface.update_custom_metric validation.""" + + @patch("poly.handlers.interface.PlatformAPIHandler.update_custom_metric") + @patch("poly.handlers.interface.PlatformAPIHandler.get_custom_metrics") + def test_expected_values_rejected_for_non_string(self, mock_get, mock_update): + """Raises ValueError when expected_values targets a non-string metric.""" + mock_get.return_value = [{"name": "SCORE", "type": "int"}] + + with self.assertRaises(ValueError) as ctx: + AgentStudioInterface.update_custom_metric( + "us", "acc1", "proj1", "SCORE", {"expected_values": ["a", "b"]} + ) + + self.assertIn("only valid for string", str(ctx.exception)) + mock_update.assert_not_called() + + @patch("poly.handlers.interface.PlatformAPIHandler.update_custom_metric") + @patch("poly.handlers.interface.PlatformAPIHandler.get_custom_metrics") + def test_expected_values_allowed_for_string(self, mock_get, mock_update): + """Does not raise when expected_values targets a string metric.""" + mock_get.return_value = [{"name": "STATUS", "type": "string"}] + mock_update.return_value = {"name": "STATUS"} + + AgentStudioInterface.update_custom_metric( + "us", "acc1", "proj1", "STATUS", {"expected_values": ["open"]} + ) + + mock_update.assert_called_once() + + @patch("poly.handlers.interface.PlatformAPIHandler.update_custom_metric") + def test_no_expected_values_skips_type_check(self, mock_update): + """When expected_values is not in data, no type lookup is made.""" + mock_update.return_value = {"name": "SCORE"} + + AgentStudioInterface.update_custom_metric( + "us", "acc1", "proj1", "SCORE", {"description": "new desc"} + ) + + mock_update.assert_called_once() + + +class ImportMetricsFromFileInterfaceTest(unittest.TestCase): + """Tests for AgentStudioInterface.import_metrics_from_file.""" + + def test_file_not_found_raises(self): + """Raises FileNotFoundError for a missing file.""" + with self.assertRaises(FileNotFoundError): + AgentStudioInterface.import_metrics_from_file( + "us", "acc1", "proj1", "/nonexistent/metrics.yaml" + ) + + @patch("builtins.open", unittest.mock.mock_open(read_data="{{invalid")) + @patch("os.path.exists", return_value=True) + def test_invalid_yaml_raises(self, _): + """Raises ValueError for unparseable YAML.""" + with self.assertRaises(ValueError) as ctx: + AgentStudioInterface.import_metrics_from_file("us", "acc1", "proj1", "bad.yaml") + + self.assertIn("Invalid YAML", str(ctx.exception)) + + @patch("poly.handlers.interface.PlatformAPIHandler.preview_metrics_import") + @patch("builtins.open", unittest.mock.mock_open(read_data="SCORE:\n type: int\n")) + @patch("os.path.exists", return_value=True) + def test_dry_run_returns_preview(self, _, mock_preview): + """In dry-run mode, returns preview without importing.""" + mock_preview.return_value = { + "would_create": ["SCORE"], + "would_skip": [], + "remote_only": [], + } + + result = AgentStudioInterface.import_metrics_from_file( + "us", "acc1", "proj1", "metrics.yaml", dry_run=True + ) + + self.assertTrue(result["dry_run"]) + self.assertEqual(result["would_create"], ["SCORE"]) + + @patch("poly.handlers.interface.PlatformAPIHandler.import_custom_metrics") + @patch("poly.handlers.interface.PlatformAPIHandler.preview_metrics_import") + @patch("builtins.open", unittest.mock.mock_open(read_data="SCORE:\n type: int\n")) + @patch("os.path.exists", return_value=True) + def test_import_returns_result_with_remote_only(self, _, mock_preview, mock_import): + """Full import merges remote_only from preview into the result.""" + mock_preview.return_value = { + "would_create": ["SCORE"], + "would_skip": [], + "remote_only": ["OLD_METRIC"], + } + mock_import.return_value = { + "metadata": {"created": ["SCORE"], "ignored": []}, + } - result = PlatformAPIHandler.preview_metrics_import( - "us", "acc1", "proj1", {"EXISTING", "NEW_ONE"} + result = AgentStudioInterface.import_metrics_from_file( + "us", "acc1", "proj1", "metrics.yaml", dry_run=False ) - self.assertEqual(result["would_create"], ["NEW_ONE"]) - self.assertEqual(result["would_skip"], ["EXISTING"]) - self.assertEqual(result["remote_only"], ["REMOTE_ONLY"]) + self.assertEqual(result["remote_only"], ["OLD_METRIC"]) + self.assertEqual(result["metadata"]["created"], ["SCORE"]) + mock_import.assert_called_once() class ValidMetricTypesTest(unittest.TestCase):