From a5639bd3e12de0cc14788b828def413e7ad9b40f Mon Sep 17 00:00:00 2001 From: Abram Date: Wed, 10 Jul 2024 16:23:47 +0100 Subject: [PATCH 1/6] refactor(backend): update 'fetch_evaluations_by_resource' to retrieve both evaluations and human evaluations for testset resource type --- .../agenta_backend/services/db_manager.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/agenta-backend/agenta_backend/services/db_manager.py b/agenta-backend/agenta_backend/services/db_manager.py index c83eb7bdc..4149d4724 100644 --- a/agenta-backend/agenta_backend/services/db_manager.py +++ b/agenta-backend/agenta_backend/services/db_manager.py @@ -2739,7 +2739,19 @@ async def fetch_evaluations_by_resource(resource_type: str, resource_ids: List[s if resource_type == "variant": query = select(EvaluationDB).filter(EvaluationDB.variant_id.in_(ids)) elif resource_type == "testset": - query = select(EvaluationDB).filter(EvaluationDB.testset_id.in_(ids)) + result_evaluations = await session.execute( + select(EvaluationDB) + .filter(EvaluationDB.testset_id.in_(ids)) + .options(load_only(EvaluationDB.id)) # type: ignore + ) + result_human_evaluations = await session.execute( + select(HumanEvaluationDB) + .filter(HumanEvaluationDB.testset_id.in_(ids)) + .options(load_only(HumanEvaluationDB.id)) # type: ignore + ) + res_evaluations = result_evaluations.scalars().all() + res_human_evaluations = result_human_evaluations.scalars().all() + return res_evaluations + res_human_evaluations elif resource_type == "evaluator_config": query = ( select(EvaluationDB) From 92dfee3e3c2c7c85196328e2636ede8121222a1f Mon Sep 17 00:00:00 2001 From: Abram Date: Wed, 10 Jul 2024 16:26:04 +0100 Subject: [PATCH 2/6] refactor (backend): add backward compatibility logic for deleted variants in human evaluations --- .../agenta_backend/models/converters.py | 13 +++++++++---- .../agenta_backend/routers/container_router.py | 15 ++++++++++++++- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/agenta-backend/agenta_backend/models/converters.py b/agenta-backend/agenta_backend/models/converters.py index 429ad1589..72a0d174d 100644 --- a/agenta-backend/agenta_backend/models/converters.py +++ b/agenta-backend/agenta_backend/models/converters.py @@ -167,12 +167,17 @@ async def human_evaluation_db_to_pydantic( variant_name = ( evaluation_variant.variant.variant_name if isinstance(evaluation_variant.variant_id, uuid.UUID) - else str(evaluation_variant.variant.variant_id) + else str(evaluation_variant.variant_id) ) variants_names.append(str(variant_name)) - variants_ids.append(str(evaluation_variant.variant.id)) - revisions.append(str(evaluation_variant.variant_revision.revision)) - variants_revision_ids.append(str(evaluation_variant.variant_revision.id)) + variants_ids.append(str(evaluation_variant.variant_id)) + variant_revision = ( + str(evaluation_variant.variant_revision.revision) + if isinstance(evaluation_variant.variant_revision_id, uuid.UUID) + else " None" + ) + revisions.append(variant_revision) + variants_revision_ids.append(str(evaluation_variant.variant_revision_id)) return HumanEvaluation( id=str(evaluation_db.id), diff --git a/agenta-backend/agenta_backend/routers/container_router.py b/agenta-backend/agenta_backend/routers/container_router.py index fb678ff9a..ea47ecc08 100644 --- a/agenta-backend/agenta_backend/routers/container_router.py +++ b/agenta-backend/agenta_backend/routers/container_router.py @@ -1,3 +1,4 @@ +import uuid import logging from typing import List, Optional, Union @@ -163,8 +164,20 @@ async def construct_app_container_url( if base_id: object_db = await db_manager.fetch_base_by_id(base_id) - elif variant_id: + elif variant_id and variant_id != "None": + # NOTE: Backward Compatibility + # --------------------------- + # When a user creates a human evaluation with a variant and later deletes the variant, + # the human evaluation page becomes inaccessible due to the backend raising a + # "'NoneType' object has no attribute 'variant_id'" error. To suppress this error, + # we will return the string "None" as the ID of the variant. + # This change ensures that users can still view their evaluations; however, + # they will no longer be able to access a deployment URL for the deleted variant. + # Therefore, we ensure that variant_id is not "None". object_db = await db_manager.fetch_app_variant_by_id(variant_id) + else: + # NOTE: required for backward compatibility + object_db = None # Check app access if isCloudEE(): From 7491df05d55e53f56483f6875e387baf95ad43a2 Mon Sep 17 00:00:00 2001 From: Abram Date: Wed, 10 Jul 2024 16:33:50 +0100 Subject: [PATCH 3/6] style (backend): format code with black@23.12.0 --- agenta-backend/agenta_backend/services/db_manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agenta-backend/agenta_backend/services/db_manager.py b/agenta-backend/agenta_backend/services/db_manager.py index 4149d4724..ac8abae88 100644 --- a/agenta-backend/agenta_backend/services/db_manager.py +++ b/agenta-backend/agenta_backend/services/db_manager.py @@ -2742,12 +2742,12 @@ async def fetch_evaluations_by_resource(resource_type: str, resource_ids: List[s result_evaluations = await session.execute( select(EvaluationDB) .filter(EvaluationDB.testset_id.in_(ids)) - .options(load_only(EvaluationDB.id)) # type: ignore + .options(load_only(EvaluationDB.id)) # type: ignore ) result_human_evaluations = await session.execute( select(HumanEvaluationDB) .filter(HumanEvaluationDB.testset_id.in_(ids)) - .options(load_only(HumanEvaluationDB.id)) # type: ignore + .options(load_only(HumanEvaluationDB.id)) # type: ignore ) res_evaluations = result_evaluations.scalars().all() res_human_evaluations = result_human_evaluations.scalars().all() From 6546a1f296fc95ddd7512378f0049538d4bdddf8 Mon Sep 17 00:00:00 2001 From: Abram Date: Wed, 10 Jul 2024 19:03:02 +0100 Subject: [PATCH 4/6] refactor(backend): update 'fetch_evaluations_by_resource' to retrieve both evaluations (human and auto) for variant resource type --- .../agenta_backend/services/db_manager.py | 36 +++++++++++++------ 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/agenta-backend/agenta_backend/services/db_manager.py b/agenta-backend/agenta_backend/services/db_manager.py index ac8abae88..99080a13f 100644 --- a/agenta-backend/agenta_backend/services/db_manager.py +++ b/agenta-backend/agenta_backend/services/db_manager.py @@ -2737,8 +2737,22 @@ async def fetch_evaluations_by_resource(resource_type: str, resource_ids: List[s async with db_engine.get_session() as session: if resource_type == "variant": - query = select(EvaluationDB).filter(EvaluationDB.variant_id.in_(ids)) - elif resource_type == "testset": + result_evaluations = await session.execute( + select(EvaluationDB) + .filter(EvaluationDB.variant_id.in_(ids)) + .options(load_only(EvaluationDB.id)) # type: ignore + ) + result_human_evaluations = await session.execute( + select(HumanEvaluationDB) + .join(HumanEvaluationVariantDB) + .filter(HumanEvaluationVariantDB.variant_id.in_(ids)) + .options(load_only(HumanEvaluationDB.id)) # type: ignore + ) + res_evaluations = result_evaluations.scalars().all() + res_human_evaluations = result_human_evaluations.scalars().all() + return res_evaluations + res_human_evaluations + + if resource_type == "testset": result_evaluations = await session.execute( select(EvaluationDB) .filter(EvaluationDB.testset_id.in_(ids)) @@ -2752,21 +2766,21 @@ async def fetch_evaluations_by_resource(resource_type: str, resource_ids: List[s res_evaluations = result_evaluations.scalars().all() res_human_evaluations = result_human_evaluations.scalars().all() return res_evaluations + res_human_evaluations - elif resource_type == "evaluator_config": + + if resource_type == "evaluator_config": query = ( select(EvaluationDB) .join(EvaluationDB.evaluator_configs) .filter(EvaluationEvaluatorConfigDB.evaluator_config_id.in_(ids)) ) - else: - raise HTTPException( - status_code=400, - detail=f"resource_type {resource_type} is not supported", - ) + result = await session.execute(query) + res = result.scalars().all() + return res - result = await session.execute(query) - res = result.scalars().all() - return res + raise HTTPException( + status_code=400, + detail=f"resource_type {resource_type} is not supported", + ) async def delete_evaluations(evaluation_ids: List[str]) -> None: From 6e0da53652664e1c71f9581bcb4ba054aa199d15 Mon Sep 17 00:00:00 2001 From: Abram Date: Wed, 10 Jul 2024 19:04:41 +0100 Subject: [PATCH 5/6] refactor (web): update handleDelete for variant to check if resource is valid for deletion --- .../Playground/Views/ParametersView.tsx | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/agenta-web/src/components/Playground/Views/ParametersView.tsx b/agenta-web/src/components/Playground/Views/ParametersView.tsx index 08ad60b80..a33ecde33 100644 --- a/agenta-web/src/components/Playground/Views/ParametersView.tsx +++ b/agenta-web/src/components/Playground/Views/ParametersView.tsx @@ -11,6 +11,7 @@ import {usePostHogAg} from "@/hooks/usePostHogAg" import {isDemo} from "@/lib/helpers/utils" import {useQueryParam} from "@/hooks/useQuery" import {dynamicComponent, dynamicService} from "@/lib/helpers/dynamic" +import {checkIfResourceValidForDeletion} from "@/lib/helpers/evaluate" const PromptVersioningDrawer: any = dynamicComponent( `PromptVersioningDrawer/PromptVersioningDrawer`, @@ -131,14 +132,24 @@ const ParametersView: React.FC = ({ }) } - const handleDelete = () => { - deleteVariant(() => { - if (variant.persistent) { - return deleteSingleVariant(variant.variantId).then(() => { - onStateChange(false) - }) - } - }) + const handleDelete = async () => { + try { + if ( + !(await checkIfResourceValidForDeletion({ + resourceType: "variant", + resourceIds: variant.variantId, + })) + ) + return + + deleteVariant(() => { + if (variant.persistent) { + return deleteSingleVariant(variant.variantId).then(() => { + onStateChange(false) + }) + } + }) + } catch {} } useEffect(() => { From cd9d4d0829068dcce178555b867fae01f223118e Mon Sep 17 00:00:00 2001 From: Abram Date: Thu, 11 Jul 2024 09:57:06 +0100 Subject: [PATCH 6/6] fix (build): resolve Type error: Type 'string' is not assignable to type 'string[]' --- agenta-web/src/components/Playground/Views/ParametersView.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agenta-web/src/components/Playground/Views/ParametersView.tsx b/agenta-web/src/components/Playground/Views/ParametersView.tsx index a33ecde33..47d5278d2 100644 --- a/agenta-web/src/components/Playground/Views/ParametersView.tsx +++ b/agenta-web/src/components/Playground/Views/ParametersView.tsx @@ -137,7 +137,7 @@ const ParametersView: React.FC = ({ if ( !(await checkIfResourceValidForDeletion({ resourceType: "variant", - resourceIds: variant.variantId, + resourceIds: [variant.variantId], })) ) return