diff --git a/CHANGELOG.md b/CHANGELOG.md index 775e3528..3db36d25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 sent. - `Evaluator.is_global` and `Evaluator.scope_projects` report the scope the API returned, after `create()`, `get()`, `list()` and `refresh()`. +- `EvaluatorPermissionError`, a `ForbiddenError` subclass, replaces the generic 403 message when creating, + updating, deleting or re-scoping an evaluator is refused, and says what is needed. Create needs the owner or + editor role on every listed project. Update and delete need it on one of the evaluator's projects. A scope change + needs it on one of the current projects and on each project added or removed. Anything on a global evaluator, + including creating one, needs the org admin role. The server response body is kept in the message. +- `Evaluator.set_scope(project_ids=[...])` and `set_scope(is_global=True)` replace an evaluator's scope through + `PUT /scorers/{scorer_id}/scope`. Promoting to global is org admin only. ### Changed @@ -68,6 +75,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `Evaluators.delete_evaluator()` and `Evaluator.refresh()` now say "not found, or not shared with your projects" + when the evaluator is missing, since the server hides evaluators outside the caller's projects. `Evaluator.get()` + still returns `None` in that case. Calls by id (`update()`, `delete()`, `set_scope()`) may raise `NotFoundError` + instead; `delete_evaluator(name=...)` and `Evaluator.delete_by_name()` raise `ValueError`. +- `Evaluator.delete()` now deletes by id. It used to delete every evaluator that shared its name, which could also hit + one the caller had no permission to delete. +- Creating an LLM or code evaluator no longer fails with an `AttributeError` when the scorer request is answered with + a validation error; the detail is raised as a `ValueError`, as it already was for decision evaluators. - The API client now knows the permission and enum values that current servers return, so reading projects and other resources no longer fails on them. For example, `Projects().get(id=...)` raised `ValueError: 'configure_rate_limit' is not a valid ControlResourceAction` (the project permission enum diff --git a/README.md b/README.md index 0199c87c..9b20886e 100644 --- a/README.md +++ b/README.md @@ -627,6 +627,34 @@ run_experiment( ) ``` +#### Evaluators + +New evaluators are scoped to the current project (decorator context, then `SPLUNK_AO_PROJECT_ID`, then +`SPLUNK_AO_PROJECT`) by default. Pass `project_ids` to choose other projects, or `is_global=True` for a global +evaluator. With no current project and no `project_ids`, the evaluator is created global, which only org admins can do. +Updating or deleting an evaluator needs the owner or editor role on one of its projects (org admin for a global +evaluator). Changing scope needs the owner or editor role on one of its current projects and on each project added or +removed; making it global, or changing the scope of a global evaluator, needs the org admin role. + +```python +from splunk_ao import EvaluatorPermissionError, LlmEvaluator + +try: + evaluator = LlmEvaluator( + name="politeness", prompt="Is the answer polite?", project_ids=[""] + ).create() + # Replace the scope: scoped to exactly these projects (is_global=True is org admins only). + evaluator.set_scope(project_ids=["", ""]) +except EvaluatorPermissionError as error: + print(error) # says which role is missing and how to fix it +``` + +`EvaluatorPermissionError` subclasses `ForbiddenError`. `Evaluator.get(name=...)` returns `None` when the evaluator +does not exist or is not shared with your projects. The server answers 404 rather than 403 for an evaluator outside +your projects on some paths. In that case `update`, `delete` and `set_scope` (which go by id) can raise `NotFoundError`, +while `delete_evaluator(name=...)` and `Evaluator.delete_by_name` (which go by name) raise a `ValueError` saying the +evaluator was not found, or not shared with your projects. + ### Sessions Sessions group related traces under the same conversation. To explicitly create diff --git a/src/splunk_ao/__init__.py b/src/splunk_ao/__init__.py index 3acf0294..354e0402 100644 --- a/src/splunk_ao/__init__.py +++ b/src/splunk_ao/__init__.py @@ -57,6 +57,7 @@ AuthenticationError, BadRequestError, ConflictError, + EvaluatorPermissionError, ForbiddenError, NotFoundError, RateLimitError, @@ -129,6 +130,7 @@ "DecisionVersionOutcomeUnknownError", "Document", "Evaluator", + "EvaluatorPermissionError", "Experiment", "ForbiddenError", "Integration", diff --git a/src/splunk_ao/evaluator.py b/src/splunk_ao/evaluator.py index 181b1fe9..140201a3 100644 --- a/src/splunk_ao/evaluator.py +++ b/src/splunk_ao/evaluator.py @@ -20,10 +20,14 @@ from splunk_ao.config import SplunkAOConfig from splunk_ao.configuration import Configuration from splunk_ao.evaluators import ( + _SCOPE_CHANGE_DENIED_HINT, DecisionAnswerOptions, DecisionVersionOutcomeUnknownError, Evaluators, + _create_denied_hint, + _edit_denied_hint, _resolve_create_scope, + _translate_forbidden, normalize_decision_answer_options, resolve_decision_output_type, validate_decision_request, @@ -31,7 +35,9 @@ from splunk_ao.resources.api.data import ( create_code_scorer_version_scorers_scorer_id_version_code_post, create_scorers_post, + delete_scorer_scorers_scorer_id_delete, get_validate_code_scorer_task_result_scorers_code_validate_task_id_get, + set_scorer_scope_scorers_scorer_id_scope_put, update_scorers_scorer_id_patch, validate_code_scorer_scorers_code_validate_post, ) @@ -49,6 +55,7 @@ ScorerTypes, TaskResultStatus, UpdateScorerRequest, + UpdateScorerScopeRequest, ) from splunk_ao.resources.models.invalid_result import InvalidResult from splunk_ao.resources.types import UNSET, File, Unset @@ -285,7 +292,7 @@ def get(cls, *, id: str | None = None, name: str | None = None) -> Evaluator | N Returns ------- Optional[Evaluator]: The metric if found (SplunkAOEvaluator, LlmEvaluator, CodeEvaluator, or DecisionEvaluator), - None otherwise. + None otherwise. ``None`` can also mean the evaluator exists but isn't shared with the caller's projects. Raises ------ @@ -384,7 +391,9 @@ def delete_by_name(cls, name: str) -> None: Raises ------ - ValueError: If no metric with the given name exists. + ValueError: If no metric with the given name exists, or it isn't shared with your projects. + EvaluatorPermissionError: If the API answers 403 (owner or editor role needed on one of the + evaluator's projects; org admin for a global evaluator). Examples -------- @@ -514,6 +523,8 @@ def update(self, **kwargs: Any) -> Evaluator: or the metric is in FAILED_SYNC state. ValueError: If any unsupported fields are passed. APIError: If the API returns a validation error or empty response. + EvaluatorPermissionError: If the API answers 403: updating needs the owner or editor role on one of the + evaluator's projects (org admin for a global evaluator). State is set to FAILED_SYNC. Exception: If the API call fails (state is set to FAILED_SYNC). Examples @@ -546,7 +557,8 @@ def update(self, **kwargs: Any) -> Evaluator: logger.info(f"Evaluator.update: id='{self.id}' name='{self.name}' - started") try: config = SplunkAOConfig.get() - response = update_scorers_scorer_id_patch.sync(scorer_id=self.id, client=config.api_client, body=body) + with _translate_forbidden(_edit_denied_hint("Updating")): + response = update_scorers_scorer_id_patch.sync(scorer_id=self.id, client=config.api_client, body=body) except Exception as e: self._set_state(SyncState.FAILED_SYNC, error=e) logger.error(f"Evaluator.update: id='{self.id}' - failed: {e}") @@ -562,16 +574,106 @@ def update(self, **kwargs: Any) -> Evaluator: logger.info(f"Evaluator.update: id='{self.id}' - completed") return self + def set_scope(self, *, project_ids: builtins.list[str] | None = None, is_global: bool = False) -> Evaluator: + """ + Replace this evaluator's scope on the API. + + This is a full replace: the evaluator ends up scoped to exactly ``project_ids``, or global. It never + falls back to the current project. On success the instance is updated with the API response and + returned in SYNCED state. + + Parameters + ---------- + project_ids : list[str], optional + Projects to scope the evaluator to. Required (and non-empty) unless ``is_global=True``. + is_global : bool + Make the evaluator global. Cannot be combined with ``project_ids``. Org admins only. + + Returns + ------- + Evaluator: This evaluator instance with updated attributes from the API. + + Raises + ------ + ValidationError: If this is a local evaluator, or the arguments are invalid (``is_global=True`` with + ``project_ids``, ``project_ids`` missing, empty or a single string). + ValueError: If the evaluator ID is not set, the evaluator is deleted, or the evaluator is in FAILED_SYNC state. + APIError: If the API returns a validation error or empty response. + EvaluatorPermissionError: If the API answers 403: changing an evaluator's scope needs the owner or + editor role on one of its current projects and on each project added or removed. Making it global, + or changing the scope of a global evaluator, needs the org admin role. + NotFoundError: If the API answers 404. The server also answers 404 for an evaluator that exists but + isn't shared with your projects, or the server is too old to support evaluator scopes. + Exception: If the API call fails (state is set to FAILED_SYNC). + + Examples + -------- + evaluator = Evaluator.get(name="factuality-checker") + evaluator.set_scope(project_ids=[""]) + evaluator.set_scope(is_global=True) # org admins only + """ + if isinstance(self, LocalEvaluator): + raise ValidationError("Local evaluators don't exist on the server and can't change scope.") + if self.id is None: + raise ValueError("Evaluator ID is not set. Cannot change the scope of a local-only evaluator.") + if self.sync_state == SyncState.DELETED: + raise ValueError("Cannot change the scope of a deleted evaluator.") + if self.sync_state == SyncState.FAILED_SYNC: + raise ValueError( + "Cannot change the scope of an evaluator in FAILED_SYNC state. " + "Call refresh() to re-sync from the API, then retry your changes." + ) + + if is_global: + if project_ids: + raise ValidationError("An evaluator cannot be both global and scoped: pass is_global=True or project_ids.") + body = UpdateScorerScopeRequest(is_global=True) + else: + # Typed as object so the str guard is not flagged unreachable for a list[str] parameter. + requested: object = project_ids + if isinstance(requested, str): + raise ValidationError("project_ids must be a list of project ids, not a single string.") + if not project_ids: + raise ValidationError("project_ids must not be empty. Pass is_global=True to make the evaluator global.") + body = UpdateScorerScopeRequest(is_global=False, project_ids=list(dict.fromkeys(project_ids))) + + logger.info(f"Evaluator.set_scope: id='{self.id}' name='{self.name}' - started") + try: + config = SplunkAOConfig.get() + with _translate_forbidden(_SCOPE_CHANGE_DENIED_HINT): + response = set_scorer_scope_scorers_scorer_id_scope_put.sync( + scorer_id=self.id, client=config.api_client, body=body + ) + except Exception as e: + self._set_state(SyncState.FAILED_SYNC, error=e) + logger.error(f"Evaluator.set_scope: id='{self.id}' - failed: {e}") + raise + + if isinstance(response, HTTPValidationError): + raise APIError(f"Evaluator scope validation error: {response.detail}") + if response is None: + raise APIError(f"Evaluator scope update returned empty response for id '{self.id}'") + + self._populate_from_scorer_response(response) + self._set_state(SyncState.SYNCED) + logger.info(f"Evaluator.set_scope: id='{self.id}' - completed") + return self + def delete(self) -> None: """ Delete this metric. - Only works for server-side metrics. Local metrics don't need deletion. + Only works for server-side metrics. Local metrics don't need deletion. Deletes by id, so a different + evaluator that shares this one's name is not touched. Raises ------ ValidationError: If this is a local metric. - ValueError: If the metric is not synced. + ValueError: If the metric ID is not set, or the API answers with a validation error or empty response. + EvaluatorPermissionError: If the API answers 403: deleting needs the owner or editor role on one of the + evaluator's projects (org admin for a global evaluator). + NotFoundError: If the API answers 404. The server also answers 404 for an evaluator that exists but + isn't shared with your projects. Examples -------- @@ -586,8 +688,13 @@ def delete(self) -> None: try: logger.info(f"Evaluator.delete: id='{self.id}' name='{self.name}' - started") - metrics_service = Evaluators() - metrics_service.delete_evaluator(name=self.name) + config = SplunkAOConfig.get() + with _translate_forbidden(_edit_denied_hint("Deleting")): + response = delete_scorer_scorers_scorer_id_delete.sync(scorer_id=self.id, client=config.api_client) + if isinstance(response, HTTPValidationError): + raise ValueError(response.detail) + if response is None: + raise ValueError("Failed to delete metric.") self._set_state(SyncState.DELETED) logger.info(f"Evaluator.delete: id='{self.id}' - completed") except Exception as e: @@ -605,7 +712,8 @@ def refresh(self) -> None: ------ ValidationError: If this is a local metric. ValueError: If the metric is not synced. - Exception: If the API call fails or the metric no longer exists. + ValueError: If the metric is not found, or not shared with your projects. + Exception: If the API call fails. Examples -------- @@ -625,7 +733,7 @@ def refresh(self) -> None: retrieved_scorer = next((s for s in scorers if s.id == self.id), None) if retrieved_scorer is None: - raise ValueError(f"Evaluator with id '{self.id}' no longer exists") + raise ValueError(f"Evaluator with id '{self.id}' not found, or not shared with your projects.") self._populate_from_scorer_response(retrieved_scorer) self._set_state(SyncState.SYNCED) @@ -847,6 +955,8 @@ def create(self) -> LlmEvaluator: ValidationError: If configuration is invalid, or the scope is invalid (``is_global=True`` with ``project_ids``, an empty ``project_ids``, or ``is_global=False`` with no project). Scope errors are ``ValidationError``, not ``ValueError``. + EvaluatorPermissionError: If the API answers 403: org admins only for a global evaluator, owner or + editor role on the projects for a scoped one. NotFoundError: If the current project is set but cannot be found. Exception: If the API call fails. @@ -1020,6 +1130,9 @@ def create(self) -> DecisionEvaluator: ValidationError: If the scope is invalid (``is_global=True`` with ``project_ids``, an empty ``project_ids``, or ``is_global=False`` with no project). Scope errors are ``ValidationError``, not ``ValueError``. + EvaluatorPermissionError: If the API answers 403: org admins only for a global evaluator, owner or + editor role on the projects for a scoped one. Decision evaluators may also not be enabled for the + organization. A 403 on the version request deletes the new scorer before the error is raised. NotFoundError: If the current project is set but cannot be found. ValueError: If an argument is invalid or the API answers with a validation error. SplunkAOAPIError: For HTTP errors. A scorer created before a clear rejection is deleted. @@ -1361,6 +1474,8 @@ def create(self) -> CodeEvaluator: ValidationError: If code is not set, validation fails, or configuration is invalid, or the scope is invalid (``is_global=True`` with ``project_ids``, an empty ``project_ids``, or ``is_global=False`` with no project). Scope errors are ``ValidationError``, not ``ValueError``. + EvaluatorPermissionError: If the API answers 403: org admins only for a global evaluator, owner or + editor role on the projects for a scoped one. NotFoundError: If the current project is set but cannot be found. Exception: If the API call fails. @@ -1416,8 +1531,11 @@ def create(self) -> CodeEvaluator: project_ids=scope_project_ids, ) - scorer_response = create_scorers_post.sync(client=config.api_client, body=scorer_request) + with _translate_forbidden(_create_denied_hint(scope_is_global)): + scorer_response = create_scorers_post.sync(client=config.api_client, body=scorer_request) + if isinstance(scorer_response, HTTPValidationError): + raise ValueError(scorer_response.detail) if scorer_response is None: logger.debug("CodeEvaluator.create: No response from create_scorers_post") raise ValueError("Failed to create code-based metric: No response from API") diff --git a/src/splunk_ao/evaluators.py b/src/splunk_ao/evaluators.py index 512da5eb..cc1274fe 100644 --- a/src/splunk_ao/evaluators.py +++ b/src/splunk_ao/evaluators.py @@ -1,11 +1,12 @@ import datetime import logging -from collections.abc import Mapping, Sequence +from collections.abc import Iterator, Mapping, Sequence +from contextlib import contextmanager from typing import Any from galileo_core.schemas.logging.step import StepType from splunk_ao.config import SplunkAOConfig -from splunk_ao.exceptions import SplunkAOAPIError +from splunk_ao.exceptions import EvaluatorPermissionError, ForbiddenError, SplunkAOAPIError from splunk_ao.resources.api.data import ( create_decision_scorer_version_scorers_scorer_id_version_decision_post, create_llm_scorer_version_scorers_scorer_id_version_llm_post, @@ -94,6 +95,60 @@ def __init__(self, scorer_id: str, cause: Exception) -> None: ) +# E2E tests match the phrases "org admin" and "owner or editor" in these hints, so keep them. +_GLOBAL_CREATE_DENIED_HINT = ( + "Only org admins can create global evaluators. Pass project_ids=[...] to scope the evaluator to your projects, " + "or set SPLUNK_AO_PROJECT / SPLUNK_AO_PROJECT_ID so it defaults to one." +) +_SCOPED_CREATE_DENIED_HINT = ( + "Creating a project-scoped evaluator needs the owner or editor role on every project in project_ids " + "(or on the current project when it is the default). Ask a project owner for access, or choose other projects." +) +_EXPLICIT_GLOBAL_CREATE_DENIED_HINT = ( + "Only org admins can create global evaluators. Drop is_global=True and pass project_ids=[...] " + "to scope the evaluator to your projects." +) +_DECISION_ENABLED_HINT = " Decision evaluators may also not be enabled for your organization." +_DECISION_VERSION_DENIED_HINT = ( + "The server refused to save the decision evaluator's configuration, so the SDK deleted the new evaluator " + "(a warning with its id is logged if that cleanup failed). " + "Check that decision evaluators are enabled for your organization." +) +_SCOPE_CHANGE_DENIED_HINT = ( + "Changing an evaluator's scope needs the owner or editor role on one of its current projects and on each " + "project added or removed. Making it global, or changing the scope of a global evaluator, needs the org admin " + "role." +) + + +def _edit_denied_hint(gerund: str) -> str: + """Hint for a 403 when updating or deleting an evaluator; ``gerund`` is "Updating" or "Deleting".""" + return ( + f"{gerund} an evaluator needs the owner or editor role on one of its projects " + "(org admin for a global evaluator)." + ) + + +def _create_denied_hint(scope_is_global: bool | Unset) -> str: + """Hint for a 403 on create, chosen from the scope that was actually sent.""" + if scope_is_global is False: + return _SCOPED_CREATE_DENIED_HINT + if scope_is_global is True: + return _EXPLICIT_GLOBAL_CREATE_DENIED_HINT + return _GLOBAL_CREATE_DENIED_HINT + + +@contextmanager +def _translate_forbidden(hint: str) -> Iterator[None]: + """Re-raise a 403 from the wrapped call as an ``EvaluatorPermissionError`` carrying ``hint``.""" + try: + yield + except EvaluatorPermissionError: + raise + except ForbiddenError as error: + raise EvaluatorPermissionError(error.status_code, error.content, hint) from error + + REJECTION_STATUS_CODES = frozenset({400, 403, 404, 422}) @@ -261,10 +316,13 @@ def __init__(self) -> None: def delete_evaluator(self, name: str) -> None: scorers_to_delete = Scorers().list(name=name) if not scorers_to_delete: - raise ValueError(f"Scorer with name {name} not found.") + raise ValueError(f"Evaluator with name '{name}' not found, or not shared with your projects.") for scorer in scorers_to_delete: - response = delete_scorer_scorers_scorer_id_delete.sync(scorer_id=scorer.id, client=self.config.api_client) + with _translate_forbidden(_edit_denied_hint("Deleting")): + response = delete_scorer_scorers_scorer_id_delete.sync( + scorer_id=scorer.id, client=self.config.api_client + ) if isinstance(response, HTTPValidationError): raise ValueError(response.detail) @@ -329,6 +387,9 @@ def create_custom_llm_evaluator( ValidationError If the scope arguments conflict, ``project_ids`` is empty, or ``is_global=False`` is passed with no project to scope to. + EvaluatorPermissionError + If the API answers 403: org admins only for a global evaluator, owner or editor role on the + projects for a scoped one. NotFoundError If the current project is set but cannot be found. """ @@ -348,7 +409,12 @@ def create_custom_llm_evaluator( project_ids=scope_project_ids, ) - scorer = create_scorers_post.sync(body=create_scorer_request, client=self.config.api_client) + with _translate_forbidden(_create_denied_hint(scope_is_global)): + scorer = create_scorers_post.sync(body=create_scorer_request, client=self.config.api_client) + if isinstance(scorer, HTTPValidationError): + raise ValueError(scorer.detail) + if scorer is None: + raise ValueError("Failed to create custom LLM evaluator.") version_req = CreateLLMScorerVersionRequest(user_prompt=user_prompt) version_resp = create_llm_scorer_version_scorers_scorer_id_version_llm_post.sync( @@ -420,14 +486,17 @@ def create_decision_evaluator( ValidationError If the scope arguments conflict, ``project_ids`` is empty, or ``is_global=False`` is passed with no project to scope to. + EvaluatorPermissionError + If the API answers 403: org admins only for a global evaluator, owner or editor role on the + projects for a scoped one. NotFoundError If the current project is set but cannot be found. ValueError If an argument is invalid or the API answers with a validation error. If the scorer was created but its version was rejected, the scorer is deleted before the error is raised. SplunkAOAPIError - For a definitive rejection (400, 403, 404, 422), for example 403 when the feature is not enabled. - The scorer is deleted before the error is raised. + For a definitive rejection of the version request (400, 404, 422). The scorer is deleted before the + error is raised. A 403 on either request is raised as ``EvaluatorPermissionError``. DecisionVersionOutcomeUnknownError For a timeout, network error, 5xx or any other failure. The version may have been saved, so the scorer is kept and the error names its id. @@ -452,7 +521,8 @@ def create_decision_evaluator( project_ids=scope_project_ids, ) - scorer = create_scorers_post.sync(body=create_scorer_request, client=self.config.api_client) + with _translate_forbidden(_create_denied_hint(scope_is_global) + _DECISION_ENABLED_HINT): + scorer = create_scorers_post.sync(body=create_scorer_request, client=self.config.api_client) if isinstance(scorer, HTTPValidationError): raise ValueError(scorer.detail) if scorer is None: @@ -469,7 +539,9 @@ def create_decision_evaluator( except Exception as error: if self._was_rejected(error): self._delete_scorer_quietly(scorer.id) - raise + # The bare raise re-raises the 403 inside the with, which translates it. + with _translate_forbidden(_DECISION_VERSION_DENIED_HINT): + raise _logger.warning( "Decision evaluator version request failed with an unknown outcome; scorer_id=%s was kept: %s", scorer.id, @@ -599,6 +671,9 @@ def create_custom_llm_evaluator( ValidationError If the scope arguments conflict, ``project_ids`` is empty, or ``is_global=False`` is passed with no project to scope to. + EvaluatorPermissionError + If the API answers 403: org admins only for a global evaluator, owner or editor role on the + projects for a scoped one. NotFoundError If the current project is set but cannot be found. """ @@ -683,14 +758,17 @@ def create_decision_evaluator( ValidationError If the scope arguments conflict, ``project_ids`` is empty, or ``is_global=False`` is passed with no project to scope to. + EvaluatorPermissionError + If the API answers 403: org admins only for a global evaluator, owner or editor role on the + projects for a scoped one. NotFoundError If the current project is set but cannot be found. ValueError If an argument is invalid or the API answers with a validation error. If the scorer was created but its version was rejected, the scorer is deleted before the error is raised. SplunkAOAPIError - For a definitive rejection (400, 403, 404, 422), for example 403 when the feature is not enabled. - The scorer is deleted before the error is raised. + For a definitive rejection of the version request (400, 404, 422). The scorer is deleted before the + error is raised. A 403 on either request is raised as ``EvaluatorPermissionError``. DecisionVersionOutcomeUnknownError For a timeout, network error, 5xx or any other failure. The version may have been saved, so the scorer is kept and the error names its id. diff --git a/src/splunk_ao/exceptions.py b/src/splunk_ao/exceptions.py index 8b1af634..bba2ece4 100644 --- a/src/splunk_ao/exceptions.py +++ b/src/splunk_ao/exceptions.py @@ -6,6 +6,7 @@ "AuthenticationError", "BadRequestError", "ConflictError", + "EvaluatorPermissionError", "ForbiddenError", "NotFoundError", "RateLimitError", @@ -65,6 +66,18 @@ def __init__(self, status_code: int, content: bytes): ) +class EvaluatorPermissionError(ForbiddenError): + """HTTP 403 on an evaluator create, update, delete or scope change. + + Subclasses ``ForbiddenError``, so existing ``except ForbiddenError`` handlers still catch it. The message says + which role or permission the operation needs; the server response body follows it. + """ + + def __init__(self, status_code: int, content: bytes, message: str) -> None: + # ForbiddenError fixes its own message, so skip it and use the base initializer to set ours. + SplunkAOAPIError.__init__(self, status_code, content, message) + + class NotFoundError(SplunkAOAPIError): r"""HTTP 404 - Resource not found. diff --git a/tests/test_evaluator.py b/tests/test_evaluator.py index 49301811..42f81298 100644 --- a/tests/test_evaluator.py +++ b/tests/test_evaluator.py @@ -502,17 +502,20 @@ def test_list_with_scorer_types_filter(self, mock_scorers_class: MagicMock, rese class TestMetricDelete: """Test suite for Evaluator.delete() method.""" - @patch("splunk_ao.evaluator.Evaluators") + @patch("splunk_ao.evaluator.SplunkAOConfig.get") + @patch("splunk_ao.evaluator.delete_scorer_scorers_scorer_id_delete") @patch("splunk_ao.evaluator.Scorers") def test_delete_removes_metric( - self, mock_scorers_class: MagicMock, mock_metrics_class: MagicMock, reset_configuration: None + self, + mock_scorers_class: MagicMock, + mock_delete: MagicMock, + mock_config_get: MagicMock, + reset_configuration: None, ) -> None: - """Test delete() removes the metric.""" + """Test delete() removes the metric by id.""" mock_scorers_service = MagicMock() mock_scorers_class.return_value = mock_scorers_service - - mock_metrics_service = MagicMock() - mock_metrics_class.return_value = mock_metrics_service + mock_delete.sync.return_value = {"message": "deleted"} metric_id = str(uuid4()) mock_scorer = MagicMock() @@ -533,7 +536,7 @@ def test_delete_removes_metric( metric = Evaluator.get(id=metric_id) metric.delete() - mock_metrics_service.delete_evaluator.assert_called_once_with(name="Test Evaluator") + mock_delete.sync.assert_called_once_with(scorer_id=metric_id, client=mock_config_get.return_value.api_client) assert metric.sync_state == SyncState.DELETED def test_delete_raises_error_for_local_only(self, reset_configuration: None) -> None: @@ -635,7 +638,7 @@ def test_refresh_raises_error_when_metric_no_longer_exists( metric = Evaluator.get(id=metric_id) - with pytest.raises(ValueError, match="no longer exists"): + with pytest.raises(ValueError, match="not found, or not shared with your projects"): metric.refresh() diff --git a/tests/test_evaluator_permissions.py b/tests/test_evaluator_permissions.py new file mode 100644 index 00000000..49e84c26 --- /dev/null +++ b/tests/test_evaluator_permissions.py @@ -0,0 +1,602 @@ +"""Clear errors for evaluator permission denials, and ``Evaluator.set_scope``.""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import MagicMock, patch +from uuid import uuid4 + +import pytest + +import splunk_ao +from splunk_ao.evaluator import CodeEvaluator, DecisionEvaluator, Evaluator, LlmEvaluator, LocalEvaluator +from splunk_ao.evaluators import Evaluators, create_custom_llm_evaluator, create_decision_evaluator +from splunk_ao.exceptions import EvaluatorPermissionError, ForbiddenError, NotFoundError +from splunk_ao.resources.models import ( + HTTPValidationError, + ScorerResponse, + ScorerScopeProjectRef, + UpdateScorerScopeRequest, +) +from splunk_ao.resources.types import UNSET +from splunk_ao.shared.base import SyncState +from splunk_ao.shared.exceptions import APIError, ValidationError + +pytestmark = pytest.mark.usefixtures("no_current_project") + +SERVER_BODY = b'{"detail": "denied by server"}' +UNSET_GLOBAL_HINT = ( + "Only org admins can create global evaluators. Pass project_ids=[...] to scope the evaluator to your projects, " + "or set SPLUNK_AO_PROJECT / SPLUNK_AO_PROJECT_ID so it defaults to one." +) +EXPLICIT_GLOBAL_HINT = "Only org admins can create global evaluators. Drop is_global=True and pass project_ids=[...]" +SCOPED_HINT = "owner or editor role on every project in project_ids" +DECISION_ENABLED_HINT = "Decision evaluators may also not be enabled for your organization." +UPDATE_HINT = "Updating an evaluator needs the owner or editor role on one of its projects (org admin for a global" +DELETE_HINT = "Deleting an evaluator needs the owner or editor role on one of its projects (org admin for a global" +SCOPE_HINT = ( + "owner or editor role on one of its current projects and on each project added or removed. " + "Making it global, or changing the scope of a global evaluator, needs the org admin role." +) +GLOBAL_CASES = [({}, UNSET_GLOBAL_HINT), ({"is_global": True}, EXPLICIT_GLOBAL_HINT)] +GLOBAL_IDS = ["unset_scope", "explicit_global"] +ALL_CREATE_CASES = [*GLOBAL_CASES, ({"project_ids": ["p1"]}, SCOPED_HINT)] +ALL_CREATE_IDS = [*GLOBAL_IDS, "scoped"] + + +def _forbidden() -> ForbiddenError: + return ForbiddenError(403, SERVER_BODY) + + +def _assert_permission_error(error: BaseException, hint: str) -> None: + assert isinstance(error, EvaluatorPermissionError) + assert isinstance(error, ForbiddenError) + assert error.status_code == 403 + assert hint in error.message + assert str(error).startswith(error.message) + assert "denied by server" in str(error) + + +def _scorer_response(scorer_id: str, **extra: Any) -> ScorerResponse: + row: dict[str, Any] = { + "id": scorer_id, + "name": "ev", + "scorer_type": "llm", + "tags": [], + "created_at": "2026-01-01T00:00:00+00:00", + "updated_at": "2026-01-01T00:00:00+00:00", + } + row.update(extra) + return ScorerResponse.from_dict(row) + + +def _synced(evaluator: Evaluator) -> Evaluator: + evaluator._sync_attrs(id=str(uuid4())) + evaluator._set_state(SyncState.SYNCED) + return evaluator + + +class TestEvaluatorPermissionErrorClass: + def test_is_forbidden_error_and_exported(self) -> None: + # Given: the error built the way the SDK builds it + # When: it is created and formatted + error = EvaluatorPermissionError(403, SERVER_BODY, "Only org admins can do this.") + + # Then: it is a ForbiddenError with the hint first and the server body after it, and it is exported + assert isinstance(error, ForbiddenError) + assert error.status_code == 403 + assert str(error).startswith("Only org admins can do this. (HTTP 403)") + assert "denied by server" in str(error) + assert "EvaluatorPermissionError" in splunk_ao.exceptions.__all__ + assert splunk_ao.EvaluatorPermissionError is EvaluatorPermissionError + assert "EvaluatorPermissionError" in splunk_ao.__all__ + + +@patch("splunk_ao.evaluators.create_llm_scorer_version_scorers_scorer_id_version_llm_post") +@patch("splunk_ao.evaluators.create_scorers_post") +class TestCreateLlmDenied: + @pytest.mark.parametrize("use_function", [False, True], ids=["method", "module_function"]) + @pytest.mark.parametrize(("kwargs", "hint"), GLOBAL_CASES, ids=GLOBAL_IDS) + def test_global_create_403_names_admins_and_the_fix( + self, mock_create_scorer, mock_create_version, use_function, kwargs, hint + ) -> None: + # Given: the server refuses a create whose scope is unset or global + mock_create_scorer.sync.side_effect = _forbidden() + create = create_custom_llm_evaluator if use_function else Evaluators().create_custom_llm_evaluator + + # When: the evaluator is created + with pytest.raises(EvaluatorPermissionError) as exc_info: + create(name="m", user_prompt="prompt", **kwargs) + + # Then: the error says only org admins can create global evaluators and how to scope, with no version request + _assert_permission_error(exc_info.value, hint) + mock_create_version.sync.assert_not_called() + + @pytest.mark.parametrize("use_function", [False, True], ids=["method", "module_function"]) + def test_scoped_create_403_names_the_project_role( + self, mock_create_scorer, mock_create_version, use_function + ) -> None: + # Given: a scoped create the server refuses + mock_create_scorer.sync.side_effect = _forbidden() + create = create_custom_llm_evaluator if use_function else Evaluators().create_custom_llm_evaluator + + # When: the evaluator is created + with pytest.raises(EvaluatorPermissionError) as exc_info: + create(name="m", user_prompt="prompt", project_ids=["p1"]) + + # Then: the message is about the project role, not about admins + _assert_permission_error(exc_info.value, SCOPED_HINT) + assert "org admins" not in str(exc_info.value) + + def test_hint_follows_the_resolved_scope_not_the_instance( + self, mock_create_scorer, mock_create_version, monkeypatch + ) -> None: + # Given: no explicit scope, but a current project that resolves the scope to that project + monkeypatch.setenv("SPLUNK_AO_PROJECT_ID", "proj-1") + mock_create_scorer.sync.side_effect = _forbidden() + + # When: the evaluator is created + with patch("splunk_ao.evaluators._resolve_project", return_value=MagicMock(id="proj-1")): + with pytest.raises(EvaluatorPermissionError) as exc_info: + Evaluators().create_custom_llm_evaluator(name="m", user_prompt="prompt") + + # Then: the scoped hint is used because the resolved scope is not global + _assert_permission_error(exc_info.value, SCOPED_HINT) + + def test_already_translated_error_is_not_wrapped_again(self, mock_create_scorer, mock_create_version) -> None: + # Given: a permission error with its own hint raised from the endpoint + original = EvaluatorPermissionError(403, b"x", "Custom hint.") + mock_create_scorer.sync.side_effect = original + + # When: the evaluator is created + with pytest.raises(EvaluatorPermissionError) as exc_info: + Evaluators().create_custom_llm_evaluator(name="m", user_prompt="prompt", is_global=True) + + # Then: the same object comes out, with no cause chained + assert exc_info.value is original + assert exc_info.value.__cause__ is None + + def test_422_is_raised_as_value_error(self, mock_create_scorer, mock_create_version) -> None: + # Given: the scorer post answers with a validation error + mock_create_scorer.sync.return_value = HTTPValidationError(detail="bad name") + + # When/Then: the detail surfaces as ValueError and no version request is made + with pytest.raises(ValueError, match="bad name"): + Evaluators().create_custom_llm_evaluator(name="m", user_prompt="prompt", is_global=True) + mock_create_version.sync.assert_not_called() + + def test_empty_response_is_raised_as_value_error(self, mock_create_scorer, mock_create_version) -> None: + # Given: the scorer post returns nothing + mock_create_scorer.sync.return_value = None + + # When/Then: a ValueError is raised instead of reading .id off None + with pytest.raises(ValueError, match="Failed to create custom LLM evaluator"): + Evaluators().create_custom_llm_evaluator(name="m", user_prompt="prompt", is_global=True) + + def test_llm_evaluator_create_keeps_the_error_and_marks_failed( + self, mock_create_scorer, mock_create_version + ) -> None: + # Given: an LlmEvaluator with the default (global) scope the server refuses + mock_create_scorer.sync.side_effect = _forbidden() + evaluator = LlmEvaluator(name="m", prompt="p") + + # When: it is created + with pytest.raises(EvaluatorPermissionError) as exc_info: + evaluator.create() + + # Then: the permission error is raised once (its cause is the plain 403) and the state is FAILED_SYNC + _assert_permission_error(exc_info.value, UNSET_GLOBAL_HINT) + assert type(exc_info.value.__cause__) is ForbiddenError + assert evaluator.sync_state == SyncState.FAILED_SYNC + + def test_llm_evaluator_create_with_projects_says_owner_or_editor( + self, mock_create_scorer, mock_create_version + ) -> None: + # Given: an LlmEvaluator scoped to a project the server refuses + mock_create_scorer.sync.side_effect = _forbidden() + + # When: it is created + with pytest.raises(EvaluatorPermissionError) as exc_info: + LlmEvaluator(name="m", prompt="p", project_ids=["p1"]).create() + + # Then: the hint names the project role + _assert_permission_error(exc_info.value, SCOPED_HINT) + + +@patch("splunk_ao.evaluators.delete_scorer_scorers_scorer_id_delete") +@patch("splunk_ao.evaluators.create_decision_scorer_version_scorers_scorer_id_version_decision_post") +@patch("splunk_ao.evaluators.create_scorers_post") +class TestCreateDecisionDenied: + @pytest.mark.parametrize("use_function", [False, True], ids=["method", "module_function"]) + @pytest.mark.parametrize(("kwargs", "hint"), ALL_CREATE_CASES, ids=ALL_CREATE_IDS) + def test_scorer_post_403_is_translated( + self, mock_create_scorer, mock_create_version, mock_delete, use_function, kwargs, hint + ) -> None: + # Given: the scorer post is refused + mock_create_scorer.sync.side_effect = _forbidden() + create = create_decision_evaluator if use_function else Evaluators().create_decision_evaluator + + # When: the decision evaluator is created + with pytest.raises(EvaluatorPermissionError) as exc_info: + create(name="tone", question="q", **kwargs) + + # Then: the hint matches the resolved scope, mentions the feature gate, and nothing else was called + _assert_permission_error(exc_info.value, hint) + assert DECISION_ENABLED_HINT in exc_info.value.message + mock_create_version.sync.assert_not_called() + mock_delete.sync.assert_not_called() + + def test_version_post_403_is_translated_and_the_scorer_is_still_deleted( + self, mock_create_scorer, mock_create_version, mock_delete + ) -> None: + # Given: the scorer is created, then the version post is refused + scorer = _scorer_response(str(uuid4()), scorer_type="decision") + mock_create_scorer.sync.return_value = scorer + mock_create_version.sync.side_effect = _forbidden() + + # When: the decision evaluator is created + with pytest.raises(EvaluatorPermissionError) as exc_info: + create_decision_evaluator(name="tone", question="q", project_ids=["p1"]) + + # Then: the error explains the refused version (not a create-scope problem) and the orphan scorer is deleted + _assert_permission_error(exc_info.value, "refused to save the decision evaluator's configuration") + assert SCOPED_HINT not in str(exc_info.value) + assert "org admins" not in str(exc_info.value) + mock_delete.sync.assert_called_once() + assert mock_delete.sync.call_args.kwargs["scorer_id"] == scorer.id + + def test_decision_evaluator_create_raises_one_permission_error( + self, mock_create_scorer, mock_create_version, mock_delete + ) -> None: + # Given: a DecisionEvaluator with the default scope the server refuses + mock_create_scorer.sync.side_effect = _forbidden() + evaluator = DecisionEvaluator(name="tone", question="q") + + # When: it is created + with pytest.raises(EvaluatorPermissionError) as exc_info: + evaluator.create() + + # Then: one permission error (cause is the plain 403) and FAILED_SYNC + _assert_permission_error(exc_info.value, UNSET_GLOBAL_HINT) + assert type(exc_info.value.__cause__) is ForbiddenError + assert evaluator.sync_state == SyncState.FAILED_SYNC + + +@patch("splunk_ao.evaluator.SplunkAOConfig.get") +@patch("splunk_ao.evaluator.create_scorers_post") +class TestCreateCodeDenied: + @pytest.mark.parametrize(("kwargs", "hint"), ALL_CREATE_CASES, ids=ALL_CREATE_IDS) + def test_403_is_translated_by_resolved_scope(self, mock_create_scorers, mock_config, kwargs, hint) -> None: + # Given: code validation passes and the scorer post is refused + mock_create_scorers.sync.side_effect = _forbidden() + evaluator = CodeEvaluator(name="c", code="def f(): pass", **kwargs) + + # When: the code evaluator is created + with patch.object(CodeEvaluator, "_validate_code", return_value="{}"): + with pytest.raises(EvaluatorPermissionError) as exc_info: + evaluator.create() + + # Then: the hint matches the resolved scope and the state is FAILED_SYNC + _assert_permission_error(exc_info.value, hint) + assert type(exc_info.value.__cause__) is ForbiddenError + assert evaluator.sync_state == SyncState.FAILED_SYNC + + def test_422_is_raised_as_value_error(self, mock_create_scorers, mock_config) -> None: + # Given: the scorer post answers with a validation error + mock_create_scorers.sync.return_value = HTTPValidationError(detail="bad code scorer") + + # When/Then: the detail surfaces instead of an AttributeError on .id + with patch.object(CodeEvaluator, "_validate_code", return_value="{}"): + with pytest.raises(ValueError, match="bad code scorer"): + CodeEvaluator(name="c", code="x", is_global=True).create() + + +class TestUpdateAndDeleteDenied: + @patch("splunk_ao.evaluator.update_scorers_scorer_id_patch") + @patch("splunk_ao.evaluator.SplunkAOConfig.get") + def test_update_403_is_translated_and_marks_failed(self, mock_config, mock_patch) -> None: + # Given: a synced evaluator the caller cannot edit + evaluator = _synced(LlmEvaluator(name="m", prompt="p")) + mock_patch.sync.side_effect = _forbidden() + + # When: it is updated + with pytest.raises(EvaluatorPermissionError) as exc_info: + evaluator.update(name="new") + + # Then: the edit hint is raised and the state is FAILED_SYNC + _assert_permission_error(exc_info.value, UPDATE_HINT) + assert evaluator.sync_state == SyncState.FAILED_SYNC + + @patch("splunk_ao.evaluator.update_scorers_scorer_id_patch") + @patch("splunk_ao.evaluator.SplunkAOConfig.get") + def test_update_404_stays_not_found(self, mock_config, mock_patch) -> None: + # Given: the server answers 404 (it hides evaluators outside the caller's projects) + evaluator = _synced(LlmEvaluator(name="m", prompt="p")) + mock_patch.sync.side_effect = NotFoundError(404, b"gone") + + # When: the evaluator is updated + with pytest.raises(NotFoundError) as exc_info: + evaluator.update(name="new") + + # Then: it stays a NotFoundError, never a permission error, and the state is FAILED_SYNC as before + assert not isinstance(exc_info.value, EvaluatorPermissionError) + assert evaluator.sync_state == SyncState.FAILED_SYNC + + @patch("splunk_ao.evaluators.delete_scorer_scorers_scorer_id_delete") + @patch("splunk_ao.scorers.Scorers.list") + def test_delete_evaluator_403_is_translated(self, mock_list, mock_delete) -> None: + # Given: a listed scorer whose delete the server refuses + mock_list.return_value = [_scorer_response(str(uuid4()))] + mock_delete.sync.side_effect = _forbidden() + + # When: it is deleted by name through the service + with pytest.raises(EvaluatorPermissionError) as exc_info: + Evaluators().delete_evaluator(name="ev") + + # Then: the delete hint is raised + _assert_permission_error(exc_info.value, DELETE_HINT) + + @patch("splunk_ao.evaluators.delete_scorer_scorers_scorer_id_delete") + @patch("splunk_ao.scorers.Scorers.list") + def test_delete_by_name_403_is_translated(self, mock_list, mock_delete) -> None: + # Given: a listed scorer whose delete the server refuses + mock_list.return_value = [_scorer_response(str(uuid4()))] + mock_delete.sync.side_effect = _forbidden() + + # When: Evaluator.delete_by_name is called + with pytest.raises(EvaluatorPermissionError) as exc_info: + Evaluator.delete_by_name("ev") + + # Then: the delete hint is raised + _assert_permission_error(exc_info.value, DELETE_HINT) + + @patch("splunk_ao.evaluator.delete_scorer_scorers_scorer_id_delete") + @patch("splunk_ao.evaluator.SplunkAOConfig.get") + def test_instance_delete_403_is_translated_and_marks_failed(self, mock_config, mock_delete) -> None: + # Given: a synced evaluator whose delete the server refuses + evaluator = _synced(LlmEvaluator(name="ev", prompt="p")) + mock_delete.sync.side_effect = _forbidden() + + # When: it is deleted + with pytest.raises(EvaluatorPermissionError) as exc_info: + evaluator.delete() + + # Then: the delete hint is raised and the state is FAILED_SYNC + _assert_permission_error(exc_info.value, DELETE_HINT) + assert evaluator.sync_state == SyncState.FAILED_SYNC + + @patch("splunk_ao.evaluator.delete_scorer_scorers_scorer_id_delete") + @patch("splunk_ao.evaluator.SplunkAOConfig.get") + @patch("splunk_ao.scorers.Scorers.list") + def test_instance_delete_sends_one_delete_for_its_own_id(self, mock_list, mock_config, mock_delete) -> None: + # Given: a synced evaluator and a same-named sibling that the name lookup would also return + evaluator = _synced(LlmEvaluator(name="ev", prompt="p")) + mock_list.return_value = [_scorer_response(evaluator.id), _scorer_response(str(uuid4()))] + mock_delete.sync.return_value = {"message": "deleted"} + + # When: the evaluator is deleted + evaluator.delete() + + # Then: exactly one DELETE goes out, for its own id, and the sibling is untouched + mock_delete.sync.assert_called_once() + assert mock_delete.sync.call_args.kwargs["scorer_id"] == evaluator.id + mock_list.assert_not_called() + assert evaluator.sync_state == SyncState.DELETED + + @patch("splunk_ao.evaluator.delete_scorer_scorers_scorer_id_delete") + @patch("splunk_ao.evaluator.SplunkAOConfig.get") + def test_instance_delete_404_stays_not_found(self, mock_config, mock_delete) -> None: + # Given: the server answers 404 (it hides evaluators outside the caller's projects) + evaluator = _synced(LlmEvaluator(name="ev", prompt="p")) + mock_delete.sync.side_effect = NotFoundError(404, b"gone") + + # When: the evaluator is deleted + with pytest.raises(NotFoundError) as exc_info: + evaluator.delete() + + # Then: it stays a NotFoundError, never a permission error, and the state is FAILED_SYNC + assert not isinstance(exc_info.value, EvaluatorPermissionError) + assert evaluator.sync_state == SyncState.FAILED_SYNC + + @patch("splunk_ao.evaluator.delete_scorer_scorers_scorer_id_delete") + @patch("splunk_ao.evaluator.SplunkAOConfig.get") + @pytest.mark.parametrize( + ("response", "message"), + [(HTTPValidationError(detail="bad id"), "bad id"), (None, "Failed to delete")], + ids=["validation_error", "empty_response"], + ) + def test_instance_delete_rejects_a_bad_response(self, mock_config, mock_delete, response, message) -> None: + # Given: the server answers with a validation error or nothing + evaluator = _synced(LlmEvaluator(name="ev", prompt="p")) + mock_delete.sync.return_value = response + + # When/Then: a ValueError is raised and the state is FAILED_SYNC + with pytest.raises(ValueError, match=message): + evaluator.delete() + assert evaluator.sync_state == SyncState.FAILED_SYNC + + @patch("splunk_ao.evaluators.delete_scorer_scorers_scorer_id_delete") + @patch("splunk_ao.scorers.Scorers.list") + def test_delete_404_stays_not_found(self, mock_list, mock_delete) -> None: + # Given: the server answers 404 to the delete + mock_list.return_value = [_scorer_response(str(uuid4()))] + mock_delete.sync.side_effect = NotFoundError(404, b"gone") + + # When: the evaluator is deleted + with pytest.raises(NotFoundError) as exc_info: + Evaluators().delete_evaluator(name="ev") + + # Then: it stays a NotFoundError, never a permission error + assert not isinstance(exc_info.value, EvaluatorPermissionError) + + +class TestNotFoundWording: + @patch("splunk_ao.scorers.Scorers.list", return_value=[]) + def test_delete_evaluator_name_not_in_list(self, mock_list) -> None: + # Given: no scorer with that name is visible + # When/Then: the error says it may simply not be shared with the caller's projects + with pytest.raises(ValueError, match="not found, or not shared with your projects"): + Evaluators().delete_evaluator(name="ghost") + + @patch("splunk_ao.evaluator.Scorers") + def test_refresh_id_not_in_list(self, mock_scorers) -> None: + # Given: a synced evaluator whose id is no longer in the visible list + evaluator = _synced(LlmEvaluator(name="m", prompt="p")) + mock_scorers.return_value.list.return_value = [] + + # When/Then: refresh says it may not be shared with the caller's projects + with pytest.raises(ValueError, match="not found, or not shared with your projects"): + evaluator.refresh() + + @patch("splunk_ao.evaluator.Scorers") + def test_get_still_returns_none_and_documents_why(self, mock_scorers) -> None: + # Given: no visible scorers + mock_scorers.return_value.list.return_value = [] + + # When/Then: get returns None by name and id, and its docstring explains the sharing case + assert Evaluator.get(name="ghost") is None + assert Evaluator.get(id="ghost-id") is None + assert "isn't shared with the caller's projects" in (Evaluator.get.__doc__ or "") + + +@patch("splunk_ao.evaluator.set_scorer_scope_scorers_scorer_id_scope_put") +@patch("splunk_ao.evaluator.SplunkAOConfig.get") +class TestSetScope: + def test_scoped_request_populates_instance_and_returns_self(self, mock_config, mock_put) -> None: + # Given: a synced evaluator created with its own (private) scope request + evaluator = _synced(LlmEvaluator(name="ev", prompt="p", project_ids=["old"])) + mock_put.sync.return_value = _scorer_response( + evaluator.id, is_global=False, scope_projects=[{"id": "a", "name": "A"}, {"id": "b", "name": "B"}] + ) + + # When: its scope is replaced, with a duplicate id in the request + result = evaluator.set_scope(project_ids=["a", "b", "a"]) + + # Then: the request is deduplicated, the response is applied, and the create-time scope is untouched + assert result is evaluator + kwargs = mock_put.sync.call_args.kwargs + assert kwargs["scorer_id"] == evaluator.id + assert kwargs["client"] is mock_config.return_value.api_client + body = kwargs["body"] + assert isinstance(body, UpdateScorerScopeRequest) + assert body.is_global is False + assert body.project_ids == ["a", "b"] + assert evaluator.is_global is False + assert [p.id for p in evaluator.scope_projects] == ["a", "b"] + assert all(isinstance(p, ScorerScopeProjectRef) for p in evaluator.scope_projects) + assert evaluator.is_synced() + assert evaluator._create_project_ids == ["old"] + assert evaluator._create_is_global is None + + def test_global_request_sends_no_project_ids(self, mock_config, mock_put) -> None: + # Given: a synced evaluator and a server that reports it global + evaluator = _synced(LlmEvaluator(name="ev", prompt="p")) + mock_put.sync.return_value = _scorer_response(evaluator.id, is_global=True, scope_projects=[]) + + # When: it is made global + evaluator.set_scope(is_global=True) + + # Then: the request carries is_global=True and no project_ids, and the response is applied + body = mock_put.sync.call_args.kwargs["body"] + assert body.is_global is True + assert body.project_ids is UNSET + assert "project_ids" not in body.to_dict() + assert evaluator.is_global is True + assert evaluator.is_synced() + + @pytest.mark.parametrize( + ("kwargs", "message"), + [ + ({"is_global": True, "project_ids": ["a"]}, "both global and scoped"), + ({"is_global": False}, "must not be empty"), + ({"project_ids": []}, "must not be empty"), + ({"project_ids": None, "is_global": False}, "must not be empty"), + ({"project_ids": "a-single-id"}, "not a single string"), + ({"project_ids": "a-single-id", "is_global": True}, "both global and scoped"), + ], + ) + def test_invalid_arguments_make_no_request_and_keep_state(self, mock_config, mock_put, kwargs, message) -> None: + # Given: a synced evaluator + evaluator = _synced(LlmEvaluator(name="ev", prompt="p")) + + # When: set_scope gets conflicting, missing, empty or bare-string project ids + with pytest.raises(ValidationError, match=message): + evaluator.set_scope(**kwargs) + + # Then: no request was made and the state is unchanged + mock_put.sync.assert_not_called() + assert evaluator.is_synced() + + def test_local_evaluator_is_rejected(self, mock_config, mock_put) -> None: + # Given: a local-only evaluator + local = LocalEvaluator(name="local", scorer_fn=lambda trace: 1.0) + + # When/Then: set_scope refuses before any request + with pytest.raises(ValidationError, match="Local evaluators"): + local.set_scope(project_ids=["a"]) + mock_put.sync.assert_not_called() + + def test_missing_id_is_rejected(self, mock_config, mock_put) -> None: + # Given: an evaluator that was never created + evaluator = LlmEvaluator(name="ev", prompt="p") + + # When/Then: set_scope refuses before any request + with pytest.raises(ValueError, match="ID is not set"): + evaluator.set_scope(project_ids=["a"]) + mock_put.sync.assert_not_called() + + @pytest.mark.parametrize("state", [SyncState.DELETED, SyncState.FAILED_SYNC]) + def test_deleted_and_failed_sync_are_rejected(self, mock_config, mock_put, state) -> None: + # Given: an evaluator in a state that cannot be changed + evaluator = _synced(LlmEvaluator(name="ev", prompt="p")) + evaluator._set_state(state) + + # When/Then: set_scope refuses before any request + with pytest.raises(ValueError, match=r"deleted|FAILED_SYNC"): + evaluator.set_scope(project_ids=["a"]) + mock_put.sync.assert_not_called() + + @pytest.mark.parametrize("kwargs", [{"project_ids": ["a"]}, {"is_global": True}], ids=["scoped", "global"]) + def test_403_is_translated_and_marks_failed(self, mock_config, mock_put, kwargs) -> None: + # Given: a synced evaluator whose scope change the server refuses + evaluator = _synced(LlmEvaluator(name="ev", prompt="p")) + mock_put.sync.side_effect = _forbidden() + + # When: its scope is changed + with pytest.raises(EvaluatorPermissionError) as exc_info: + evaluator.set_scope(**kwargs) + + # Then: the scope-change hint is raised and the state is FAILED_SYNC + _assert_permission_error(exc_info.value, SCOPE_HINT) + assert evaluator.sync_state == SyncState.FAILED_SYNC + + def test_404_stays_not_found(self, mock_config, mock_put) -> None: + # Given: the server answers 404 (it hides evaluators outside the caller's projects) + evaluator = _synced(LlmEvaluator(name="ev", prompt="p")) + mock_put.sync.side_effect = NotFoundError(404, b"gone") + + # When: its scope is changed + with pytest.raises(NotFoundError) as exc_info: + evaluator.set_scope(project_ids=["a"]) + + # Then: it stays a NotFoundError, never a permission error, and the state is FAILED_SYNC + assert not isinstance(exc_info.value, EvaluatorPermissionError) + assert evaluator.sync_state == SyncState.FAILED_SYNC + + def test_validation_error_response_raises_api_error(self, mock_config, mock_put) -> None: + # Given: the server answers with a validation error + evaluator = _synced(LlmEvaluator(name="ev", prompt="p")) + mock_put.sync.return_value = HTTPValidationError(detail="unknown project") + + # When/Then: an APIError carries the detail + with pytest.raises(APIError, match="unknown project"): + evaluator.set_scope(project_ids=["a"]) + + def test_empty_response_raises_api_error(self, mock_config, mock_put) -> None: + # Given: the server returns nothing + evaluator = _synced(LlmEvaluator(name="ev", prompt="p")) + mock_put.sync.return_value = None + + # When/Then: an APIError reports the empty response + with pytest.raises(APIError, match="empty response"): + evaluator.set_scope(project_ids=["a"]) diff --git a/tests/test_evaluators.py b/tests/test_evaluators.py index d7cb067f..ede1f6c9 100644 --- a/tests/test_evaluators.py +++ b/tests/test_evaluators.py @@ -279,7 +279,9 @@ def test_delete_metric_not_found(self, mock_list_scorers) -> None: mock_list_scorers.return_value = [] metrics = Evaluators() - with pytest.raises(ValueError, match="Scorer with name test_metric not found."): + with pytest.raises( + ValueError, match="Evaluator with name .test_metric. not found, or not shared with your projects" + ): metrics.delete_evaluator(name="test_metric") @patch("splunk_ao.evaluators.delete_scorer_scorers_scorer_id_delete")