Merge pull request #1244 from Agenta-AI/fix/1182-prevent-used-resourc…

…es-from-deletion

Fix: 1182 prevent used resources from deletion

agenta-backend/agenta_backend/models/api/evaluation_model.py

@@ -1,6 +1,6 @@
 from enum import Enum
 from datetime import datetime
-from pydantic import BaseModel, Field
+from pydantic import BaseModel
 from typing import Optional, List, Dict, Any, Union
 from agenta_backend.models.api.api_models import Result
 
@@ -39,7 +39,7 @@ class EvaluationScenarioStatusEnum(str, Enum):
 
 
 class AggregatedResult(BaseModel):
-    evaluator_config: EvaluatorConfig
+    evaluator_config: Union[EvaluatorConfig, Dict[str, Any]]
     result: Result
 
 
@@ -66,8 +66,8 @@ class Evaluation(BaseModel):
     variant_names: List[str]
     variant_revision_ids: List[str]
     revisions: List[str]
-    testset_id: str
-    testset_name: str
+    testset_id: Optional[str]
+    testset_name: Optional[str]
     status: Result
     aggregated_results: List[AggregatedResult]
     created_at: datetime

agenta-backend/agenta_backend/models/converters.py

@@ -59,6 +59,7 @@
 )
 
 import logging
+from beanie import Link
 
 logger = logging.getLogger(__name__)
 logger.setLevel(logging.DEBUG)
@@ -87,6 +88,9 @@ async def evaluation_db_to_pydantic(
         str(evaluation_db.variant_revision)
     )
     revision = str(variant_revision.revision)
+    aggregated_results = await aggregated_result_to_pydantic(
+        evaluation_db.aggregated_results
+    )
     return Evaluation(
         id=str(evaluation_db.id),
         app_id=str(evaluation_db.app.id),
@@ -97,11 +101,15 @@ async def evaluation_db_to_pydantic(
         variant_revision_ids=[str(evaluation_db.variant_revision)],
         revisions=[revision],
         variant_names=[variant_name],
-        testset_id=str(evaluation_db.testset.id),
-        testset_name=evaluation_db.testset.name,
-        aggregated_results=await aggregated_result_to_pydantic(
-            evaluation_db.aggregated_results
+        testset_id=(
+            "" if type(evaluation_db.testset) is Link else str(evaluation_db.testset.id)
+        ),
+        testset_name=(
+            ""
+            if type(evaluation_db.testset) is Link
+            else str(evaluation_db.testset.name)
         ),
+        aggregated_results=aggregated_results,
         created_at=evaluation_db.created_at,
         updated_at=evaluation_db.updated_at,
     )
@@ -132,13 +140,19 @@ async def human_evaluation_db_to_pydantic(
         evaluation_type=evaluation_db.evaluation_type,
         variant_ids=[str(variant) for variant in evaluation_db.variants],
         variant_names=variant_names,
+        testset_id=(
+            "" if type(evaluation_db.testset) is Link else str(evaluation_db.testset.id)
+        ),
+        testset_name=(
+            ""
+            if type(evaluation_db.testset) is Link
+            else str(evaluation_db.testset.name)
+        ),
         variants_revision_ids=[
             str(variant_revision)
             for variant_revision in evaluation_db.variants_revisions
         ],
         revisions=revisions,
-        testset_id=str(evaluation_db.testset.id),
-        testset_name=evaluation_db.testset.name,
         created_at=evaluation_db.created_at,
         updated_at=evaluation_db.updated_at,
     )
@@ -171,7 +185,11 @@ async def aggregated_result_to_pydantic(results: List[AggregatedResult]) -> List
         )
         transformed_results.append(
             {
-                "evaluator_config": json.loads(evaluator_config_dict),
+                "evaluator_config": (
+                    {}
+                    if evaluator_config_dict is None
+                    else json.loads(evaluator_config_dict)
+                ),
                 "result": result.result.dict(),
             }
         )

agenta-backend/agenta_backend/routers/evaluation_router.py

@@ -3,8 +3,8 @@
 from typing import Any, List
 
 from fastapi.responses import JSONResponse
-from fastapi.encoders import jsonable_encoder
-from fastapi import HTTPException, Request, status, Response
+from fastapi import HTTPException, Request, status, Response, Query
+from beanie import PydanticObjectId as ObjectId
 
 from agenta_backend.utils.common import APIRouter
 from agenta_backend.models.api.evaluation_model import (
@@ -36,6 +36,46 @@
 router = APIRouter()
 
 
+@router.get(
+    "/by_resource/",
+    response_model=List[ObjectId],
+)
+async def fetch_evaluation_ids(
+    app_id: str,
+    resource_type: str,
+    request: Request,
+    resource_ids: List[str] = Query(None),
+):
+    """Fetches evaluation ids for a given resource type and id.
+
+    Arguments:
+        app_id (str): The ID of the app for which to fetch evaluations.
+        resource_type (str): The type of resource for which to fetch evaluations.
+        resource_ids List[ObjectId]: The IDs of resource for which to fetch evaluations.
+
+    Raises:
+        HTTPException: If the resource_type is invalid or access is denied.
+
+    Returns:
+        List[str]: A list of evaluation ids.
+    """
+    user_org_data: dict = await get_user_and_org_id(request.state.user_id)
+    access_app = await check_access_to_app(
+        user_org_data=user_org_data,
+        app_id=app_id,
+    )
+    if not access_app:
+        raise HTTPException(
+            status_code=403,
+            detail=f"You do not have access to this app: {str(app_id)}",
+        )
+
+    evaluations = await evaluation_service.fetch_evaluations_by_resource(
+        resource_type, resource_ids
+    )
+    return list(map(lambda x: x.id, evaluations))
+
+
 @router.post("/", response_model=List[Evaluation], operation_id="create_evaluation")
 async def create_evaluation(
     payload: NewEvaluation,

agenta-backend/agenta_backend/services/evaluation_service.py

@@ -35,6 +35,7 @@
 )
 
 from beanie import PydanticObjectId as ObjectId
+from beanie.operators import In
 
 
 logger = logging.getLogger(__name__)
@@ -817,3 +818,24 @@ def remove_duplicates(csvdata):
             unique_entries.append(entry)
 
     return unique_entries
+
+
+async def fetch_evaluations_by_resource(resource_type: str, resource_ids: List[str]):
+    ids = list(map(lambda x: ObjectId(x), resource_ids))
+    if resource_type == "variant":
+        res = await EvaluationDB.find(In(EvaluationDB.variant, ids)).to_list()
+    elif resource_type == "testset":
+        res = await EvaluationDB.find(In(EvaluationDB.testset.id, ids)).to_list()
+    elif resource_type == "evaluator_config":
+        res = await EvaluationDB.find(
+            In(
+                EvaluationDB.evaluators_configs,
+                ids,
+            )
+        ).to_list()
+    else:
+        raise HTTPException(
+            status_code=400,
+            detail=f"resource_type {resource_type} is not supported",
+        )
+    return res

agenta-web/src/components/Playground/Playground.tsx

@@ -15,6 +15,7 @@ import {arrayMove, SortableContext, horizontalListSortingStrategy} from "@dnd-ki
 import DraggableTabNode from "../DraggableTabNode/DraggableTabNode"
 import {useLocalStorage} from "usehooks-ts"
 import TestContextProvider from "./TestContextProvider"
+import {checkIfResourceValidForDeletion} from "@/lib/helpers/evaluate"
 import ResultComponent from "../ResultComponent/ResultComponent"
 
 const Playground: React.FC = () => {
@@ -206,6 +207,17 @@ const Playground: React.FC = () => {
             },
             onOk: async () => {
                 try {
+                    const variantId =
+                        variants.find((item) => item.variantId === tabID.current)?.variantId ||
+                        variants.find((item) => item.variantName === activeKey)?.variantId
+                    if (
+                        variantId &&
+                        !(await checkIfResourceValidForDeletion({
+                            resourceType: "variant",
+                            resourceIds: [variantId],
+                        }))
+                    )
+                        return
                     if (deleteAction) await deleteAction()
                     removeTab()
                     messageApi.open({

agenta-web/src/components/pages/evaluations/evaluationScenarios/EvaluationScenarios.tsx

@@ -101,14 +101,14 @@ const EvaluationScenarios: React.FC<Props> = () => {
         })
         scenarios[0]?.evaluators_configs.forEach((config, index) => {
             colDefs.push({
-                headerName: config.name,
+                headerName: config?.name,
                 headerComponent: (props: any) => {
-                    const evaluator = evaluators.find((item) => item.key === config.evaluator_key)!
+                    const evaluator = evaluators.find((item) => item.key === config?.evaluator_key)!
                     return (
                         <AgCustomHeader {...props}>
                             <Space direction="vertical" style={{padding: "0.5rem 0"}}>
                                 <span>{config.name}</span>
-                                <Tag color={evaluator.color}>{evaluator.name}</Tag>
+                                <Tag color={evaluator?.color}>{evaluator?.name}</Tag>
                             </Space>
                         </AgCustomHeader>
                     )

agenta-web/src/components/pages/evaluations/evaluators/EvaluatorCard.tsx

@@ -9,6 +9,7 @@ import AlertPopup from "@/components/AlertPopup/AlertPopup"
 import {deleteEvaluatorConfig} from "@/services/evaluations"
 import {useAtom} from "jotai"
 import {evaluatorsAtom} from "@/lib/atoms/evaluation"
+import {checkIfResourceValidForDeletion} from "@/lib/helpers/evaluate"
 
 const useStyles = createUseStyles((theme: JSSTheme) => ({
     card: {
@@ -60,14 +61,23 @@ const EvaluatorCard: React.FC<Props> = ({evaluatorConfig, onEdit, onSuccessDelet
     const [evaluators] = useAtom(evaluatorsAtom)
     const evaluator = evaluators.find((item) => item.key === evaluatorConfig.evaluator_key)!
 
-    const onDelete = () => {
+    const onDelete = async () => {
         AlertPopup({
             title: "Delete evaluator",
             message: "Are you sure you want to delete this evaluator?",
-            onOk: () =>
-                deleteEvaluatorConfig(evaluatorConfig.id)
-                    .then(onSuccessDelete)
-                    .catch(console.error),
+            onOk: async () => {
+                if (
+                    !(await checkIfResourceValidForDeletion({
+                        resourceType: "evaluator_config",
+                        resourceIds: [evaluatorConfig.id],
+                    }))
+                )
+                    return
+                try {
+                    await deleteEvaluatorConfig(evaluatorConfig.id)
+                    onSuccessDelete?.()
+                } catch (error) {}
+            },
         })
     }
 

agenta-web/src/lib/helpers/evaluate.ts

@@ -8,6 +8,9 @@ import {
     EvaluationScenario,
 } from "../Types"
 import {convertToCsv, downloadCsv} from "./fileManipulations"
+import {fetchEvaluatonIdsByResource} from "@/services/evaluations"
+import {getAppValues} from "@/contexts/app.context"
+import AlertPopup from "@/components/AlertPopup/AlertPopup"
 import {capitalize, round} from "lodash"
 import dayjs from "dayjs"
 import {runningStatuses} from "@/components/pages/evaluations/cellRenderers/cellRenderers"
@@ -231,6 +234,33 @@ export const getVotesPercentage = (record: HumanEvaluationListTableDataType, ind
     return record.votesData.variants_votes_data[variant]?.percentage
 }
 
+export const checkIfResourceValidForDeletion = async (
+    data: Omit<Parameters<typeof fetchEvaluatonIdsByResource>[0], "appId">,
+) => {
+    const appId = getAppValues().currentApp?.app_id
+    if (!appId) return false
+
+    const response = await fetchEvaluatonIdsByResource({...data, appId})
+    if (response.data.length > 0) {
+        const name =
+            (data.resourceType === "testset"
+                ? "Testset"
+                : data.resourceType === "evaluator_config"
+                  ? "Evaluator"
+                  : "Variant") + (data.resourceIds.length > 1 ? "s" : "")
+
+        const suffix = response.data.length > 1 ? "s" : ""
+        AlertPopup({
+            title: `${name} is in use`,
+            message: `The ${name} is currently in used by ${response.data.length} evaluation${suffix}. Please delete the evaluation${suffix} first.`,
+            cancelText: null,
+            okText: "Ok",
+        })
+        return false
+    }
+    return true
+}
+
 export function getTypedValue(res?: TypedValue) {
     const {value, type, error} = res || {}
     if (type === "error") {

agenta-web/src/lib/services/api.ts

@@ -284,7 +284,16 @@ export async function updateTestset(testsetId: String, testsetName: string, test
     return response
 }
 
-export const loadTestset = async (testsetId: string) => {
+export const loadTestset = async (testsetId: string | null) => {
+    if (!testsetId) {
+        return {
+            id: undefined,
+            name: "No Test Set Associated",
+            created_at: "",
+            updated_at: "",
+            csvdata: [],
+        }
+    }
     const response = await axios.get(`${getAgentaApiUrl()}/api/testsets/${testsetId}/`)
     return response.data
 }

agenta-web/src/pages/apps/[app_id]/testsets/index.tsx

@@ -10,6 +10,7 @@ import {deleteTestsets, useLoadTestsetsList} from "@/lib/services/api"
 import {createUseStyles} from "react-jss"
 import {testset} from "@/lib/Types"
 import {isDemo} from "@/lib/helpers/utils"
+import {checkIfResourceValidForDeletion} from "@/lib/helpers/evaluate"
 
 const useStyles = createUseStyles({
     container: {
@@ -71,6 +72,13 @@ export default function Testsets() {
     const onDelete = async () => {
         const testsetsIds = selectedRowKeys.map((key) => key.toString())
         try {
+            if (
+                !(await checkIfResourceValidForDeletion({
+                    resourceType: "testset",
+                    resourceIds: testsetsIds,
+                }))
+            )
+                return
             await deleteTestsets(testsetsIds)
             mutate()
             setSelectedRowKeys([])

agenta-web/src/services/evaluations/index.ts

@@ -207,7 +207,7 @@ export const updateAnnotationScenario = async (
 // Comparison
 export const fetchAllComparisonResults = async (evaluationIds: string[]) => {
     const scenarioGroups = await Promise.all(evaluationIds.map(fetchAllEvaluationScenarios))
-    const testset: TestSet = await loadTestset(scenarioGroups[0][0].evaluation.testset.id)
+    const testset: TestSet = await loadTestset(scenarioGroups[0][0].evaluation?.testset?.id)
 
     const inputsNameSet = new Set<string>()
     scenarioGroups.forEach((group) => {
@@ -267,3 +267,21 @@ export const fetchAllComparisonResults = async (evaluationIds: string[]) => {
         evaluations: scenarioGroups.map((group) => group[0].evaluation),
     }
 }
+
+// Evaluation IDs by resource
+export const fetchEvaluatonIdsByResource = async ({
+    resourceIds,
+    resourceType,
+    appId,
+}: {
+    resourceIds: string[]
+    resourceType: "testset" | "evaluator_config" | "variant"
+    appId: string
+}) => {
+    return axios.get(`/api/evaluations/by_resource`, {
+        params: {resource_ids: resourceIds, resource_type: resourceType, app_id: appId},
+        paramsSerializer: {
+            indexes: null, //no brackets in query params
+        },
+    })
+}