From 9e7f6c4cbd3bc985e34e3e30511288308b4002b4 Mon Sep 17 00:00:00 2001 From: Prashant Vasudevan <71649489+vprashrex@users.noreply.github.com> Date: Thu, 10 Sep 2026 02:13:06 +0530 Subject: [PATCH 1/4] feat(assessment): durable result files and own submission table - Callback envelope now carries presigned result-file URLs and the failure reason; both were hardcoded null. - Every provider batch dump is recorded on assessment.result_files, plus an errors.jsonl assembled from run, row and OpenAI error-file failures. - Assessment gets its own assessment_submission table; evaluation_dataset is no longer touched by the assessment domain. - BATCH input takes rows inline or by submission_doc_id, exactly one of the two. - Submission rows move out of Postgres into object storage, loaded only when a stage is submitted. - Legacy cron no longer polls API-created runs, which it corrupted so callbacks never fired; deterministic errors now fail the run instead of looping. - Gemini structured output no longer sends a duplicate ordering key; Anthropic effort and thinking are mapped instead of dropped. --- ...assessment_submissions_and_result_files.py | 160 +++++ .../app/api/routes/assessment/assessments.py | 12 +- backend/app/api/routes/assessment/datasets.py | 120 ++-- backend/app/api/routes/assessment/runs.py | 18 +- backend/app/core/batch/operations.py | 22 +- backend/app/core/batch/polling.py | 48 +- backend/app/crud/assessment/__init__.py | 20 +- backend/app/crud/assessment/api.py | 53 +- backend/app/crud/assessment/batch.py | 64 +- backend/app/crud/assessment/core.py | 4 +- backend/app/crud/assessment/cron.py | 13 +- backend/app/crud/assessment/dataset.py | 163 ----- backend/app/crud/assessment/submission.py | 166 ++++++ backend/app/models/assessment/__init__.py | 22 +- backend/app/models/assessment/assessment.py | 69 ++- .../app/models/assessment/assessment_api.py | 36 +- backend/app/models/assessment/submission.py | 82 +++ backend/app/models/batch_job.py | 12 + backend/app/models/llm/request.py | 11 + backend/app/services/assessment/api/batch.py | 128 +++- .../app/services/assessment/api/callbacks.py | 29 +- .../services/assessment/api/result_files.py | 322 ++++++++++ .../app/services/assessment/api/results.py | 15 +- .../app/services/assessment/api/submission.py | 75 ++- .../assessment/api/submission_store.py | 70 +++ backend/app/services/assessment/mappers.py | 44 +- backend/app/services/assessment/service.py | 33 +- backend/app/services/assessment/stages.py | 19 +- .../assessment/{dataset.py => submission.py} | 114 ++-- backend/app/services/assessment/tasks.py | 20 +- .../app/services/assessment/utils/export.py | 36 +- backend/app/services/llm/mappers.py | 9 +- backend/app/tests/assessment/test_api_crud.py | 67 +++ backend/app/tests/assessment/test_cron.py | 133 ++++- backend/app/tests/assessment/test_mappers.py | 72 ++- backend/app/tests/assessment/test_pipeline.py | 10 + .../app/tests/assessment/test_result_files.py | 564 ++++++++++++++++++ backend/app/tests/core/batch/test_polling.py | 128 ++++ .../app/tests/services/llm/test_mappers.py | 22 + docs/wiki/domain-map.md | 5 +- docs/wiki/modules/assessment.md | 25 +- docs/wiki/modules/llm-call.md | 1 + docs/wiki/modules/platform.md | 2 +- 43 files changed, 2515 insertions(+), 523 deletions(-) create mode 100644 backend/app/alembic/versions/083_assessment_submissions_and_result_files.py delete mode 100644 backend/app/crud/assessment/dataset.py create mode 100644 backend/app/crud/assessment/submission.py create mode 100644 backend/app/models/assessment/submission.py create mode 100644 backend/app/services/assessment/api/result_files.py create mode 100644 backend/app/services/assessment/api/submission_store.py rename backend/app/services/assessment/{dataset.py => submission.py} (70%) create mode 100644 backend/app/tests/assessment/test_result_files.py create mode 100644 backend/app/tests/core/batch/test_polling.py diff --git a/backend/app/alembic/versions/083_assessment_submissions_and_result_files.py b/backend/app/alembic/versions/083_assessment_submissions_and_result_files.py new file mode 100644 index 000000000..7940c583b --- /dev/null +++ b/backend/app/alembic/versions/083_assessment_submissions_and_result_files.py @@ -0,0 +1,160 @@ +"""Assessment submissions table, submission/result-file pointers, provider error file id + +Revision ID: 083 +Revises: 082 +Create Date: 2026-09-09 00:00:00.000000 + +Assessment submissions leave `evaluation_dataset`, whose type-agnostic name uniqueness +let an eval dataset block an assessment one. Multi-MB payloads leave Postgres too: +`submission_input` and `result_files` hold s3:// urls, and `provider_error_file_id` +""" + +import sqlalchemy as sa +from alembic import op +from sqlalchemy.dialects import postgresql + +revision = "083" +down_revision = "082" +branch_labels = None +depends_on = None + +RESULT_FILES_CHECK = "ck_assessment_result_files_is_object" + + +def upgrade() -> None: + op.create_table( + "assessment_submission", + sa.Column( + "id", + postgresql.UUID(as_uuid=True), + primary_key=True, + comment="Unique identifier for the submission", + ), + sa.Column( + "name", + sa.String(), + nullable=False, + comment="Sanitized name; the object key is derived from it", + ), + sa.Column( + "description", sa.String(), nullable=True, comment="Optional description" + ), + sa.Column( + "object_store_url", + sa.String(), + nullable=False, + comment="Object-store url of the uploaded file; its suffix gives the format", + ), + sa.Column( + "total_items", + sa.Integer(), + nullable=False, + server_default="0", + comment="Row count, excluding the header", + ), + sa.Column( + "organization_id", + sa.Integer(), + sa.ForeignKey("organization.id", ondelete="CASCADE"), + nullable=False, + ), + sa.Column( + "project_id", + sa.Integer(), + sa.ForeignKey("project.id", ondelete="CASCADE"), + nullable=False, + ), + sa.Column("inserted_at", sa.DateTime(), nullable=False), + sa.Column("updated_at", sa.DateTime(), nullable=False), + sa.UniqueConstraint( + "name", + "organization_id", + "project_id", + name="uq_assessment_submission_name_org_project", + ), + ) + op.create_index("ix_assessment_submission_name", "assessment_submission", ["name"]) + + op.add_column( + "assessment", + sa.Column( + "result_files", + postgresql.JSONB(astext_type=sa.Text()), + nullable=False, + server_default=sa.text("'{}'::jsonb"), + comment=( + "Result-file kind (results / errors / _results) to " + "{object_store_url} for every provider batch dump held; raw s3:// in the " + "column, presigned per delivery in the BATCH callback" + ), + ), + ) + op.add_column( + "assessment", + sa.Column( + "submission_input", + sa.String(), + nullable=True, + comment=( + "Object-store url of the API-client BATCH submission rows " + "(submission.jsonl); the rows are never stored in this table" + ), + ), + ) + op.drop_column("assessment", "dataset_id") + op.add_column( + "assessment", + sa.Column( + "submission_id", + postgresql.UUID(as_uuid=True), + sa.ForeignKey("assessment_submission.id", ondelete="SET NULL"), + nullable=True, + comment=( + "Uploaded submission the rows came from; set by RUN and by a BATCH " + "submitted with `submission_doc_id`. NULL when BATCH sent rows inline" + ), + ), + ) + op.create_index("ix_assessment_submission_id", "assessment", ["submission_id"]) + + op.add_column( + "batch_job", + sa.Column( + "provider_error_file_id", + sa.String(), + nullable=True, + comment=( + "Provider's error file ID (OpenAI only; Anthropic and Gemini report " + "per-item errors inline)" + ), + ), + ) + op.create_check_constraint( + RESULT_FILES_CHECK, + "assessment", + "jsonb_typeof(result_files) = 'object'", + ) + + +def downgrade() -> None: + op.drop_constraint(RESULT_FILES_CHECK, "assessment", type_="check") + op.drop_column("batch_job", "provider_error_file_id") + + op.drop_index("ix_assessment_submission_id", table_name="assessment") + op.drop_column("assessment", "submission_id") + op.add_column( + "assessment", + sa.Column( + "dataset_id", + sa.Integer(), + sa.ForeignKey("evaluation_dataset.id", ondelete="SET NULL"), + nullable=True, + comment="External dataset (RUN); binding lives in `input`", + ), + ) + + op.drop_column("assessment", "submission_input") + op.drop_column("assessment", "result_files") + + op.drop_index("ix_assessment_submission_name", table_name="assessment_submission") + op.drop_table("assessment_submission") diff --git a/backend/app/api/routes/assessment/assessments.py b/backend/app/api/routes/assessment/assessments.py index 287fabb94..ce1201fa2 100644 --- a/backend/app/api/routes/assessment/assessments.py +++ b/backend/app/api/routes/assessment/assessments.py @@ -1,6 +1,6 @@ """Parent-assessment endpoints (LEGACY RUN pipeline). -Serves dataset-based RUN assessments only. The new API-client BATCH path +Serves submission-based RUN assessments only. The new API-client BATCH path (`api.py`) delivers results by webhook and never surfaces here. """ @@ -28,7 +28,7 @@ AssessmentPublic, AssessmentResponse, ) -from app.models.evaluation import EvaluationDataset +from app.models.assessment import AssessmentSubmission from app.services.assessment.service import retry_assessment as retry_assessment_service from app.services.assessment.utils import build_assessment_results_response from app.utils import APIResponse, load_description @@ -47,12 +47,12 @@ def _build_assessment_public( session=session, assessment_id=assessment.id ) counts = compute_run_counts(runs) - dataset = session.get(EvaluationDataset, assessment.dataset_id) + submission = session.get(AssessmentSubmission, assessment.submission_id) return AssessmentPublic( id=assessment.id, experiment_name=assessment.experiment_name, - dataset_id=assessment.dataset_id, - dataset_name=dataset.name if dataset else None, + submission_id=assessment.submission_id, + submission_name=submission.name if submission else None, status=assessment.status, counts=counts, run_stats=build_run_stats(runs), @@ -76,7 +76,7 @@ def retry_assessment( session: SessionDep, auth_context: AuthContextDep, ) -> APIResponse[AssessmentResponse]: - """Retry a parent assessment using the same dataset/config inputs.""" + """Retry a parent assessment using the same submission/config inputs.""" assessment = get_assessment_by_id( session=session, assessment_id=assessment_id, diff --git a/backend/app/api/routes/assessment/datasets.py b/backend/app/api/routes/assessment/datasets.py index 22f000c4d..7ac4ea8dc 100644 --- a/backend/app/api/routes/assessment/datasets.py +++ b/backend/app/api/routes/assessment/datasets.py @@ -1,27 +1,25 @@ -"""Assessment dataset endpoints.""" +"""Assessment submission-file endpoints.""" import logging from typing import Annotated +from uuid import UUID from fastapi import APIRouter, Depends, File, Form, HTTPException, Query, UploadFile from app.api.deps import AuthContextDep, SessionDep from app.api.permissions import Permission, require_permission from app.core.cloud import get_cloud_storage -from app.crud.assessment.dataset import ( - delete_assessment_dataset, - get_assessment_dataset_by_id, - list_assessment_datasets, +from app.crud.assessment.submission import ( + delete_submission, + get_submission_by_id, + list_submissions, ) from app.models.assessment import ( - AssessmentDatasetPreview, - AssessmentDatasetResponse, + AssessmentSubmission, + AssessmentSubmissionPreview, + AssessmentSubmissionResponse, ) -from app.models.evaluation import EvaluationDataset -from app.services.assessment.dataset import ( - preview_dataset as preview_assessment_dataset, -) -from app.services.assessment.dataset import upload_dataset as upload_assessment_dataset +from app.services.assessment.submission import preview_submission, upload_submission from app.services.assessment.validators import validate_dataset_file from app.utils import APIResponse, load_description @@ -30,19 +28,17 @@ router = APIRouter() -def _dataset_to_response( - dataset: EvaluationDataset, +def _submission_to_response( + submission: AssessmentSubmission, signed_url: str | None = None, - preview: AssessmentDatasetPreview | None = None, -) -> AssessmentDatasetResponse: - metadata = dataset.dataset_metadata or {} - return AssessmentDatasetResponse( - dataset_id=dataset.id, - dataset_name=dataset.name, - description=dataset.description, - total_items=metadata.get("total_items_count", 0), - file_extension=metadata.get("file_extension"), - object_store_url=dataset.object_store_url, + preview: AssessmentSubmissionPreview | None = None, +) -> AssessmentSubmissionResponse: + return AssessmentSubmissionResponse( + submission_id=submission.id, + name=submission.name, + description=submission.description, + total_items=submission.total_items, + object_store_url=submission.object_store_url, signed_url=signed_url, preview=preview, ) @@ -51,50 +47,48 @@ def _dataset_to_response( @router.post( "/datasets", description=load_description("assessment/upload_dataset.md"), - response_model=APIResponse[AssessmentDatasetResponse], + response_model=APIResponse[AssessmentSubmissionResponse], dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], ) async def upload_dataset( session: SessionDep, auth_context: AuthContextDep, - file: UploadFile = File( - ..., description="CSV or Excel file to upload as a dataset" - ), - dataset_name: str = Form(..., description="Name for the dataset"), - description: str | None = Form(None, description="Optional dataset description"), -) -> APIResponse[AssessmentDatasetResponse]: - """Upload an assessment dataset (any CSV/Excel file, no column requirements).""" + file: UploadFile = File(..., description="CSV or Excel file to upload"), + dataset_name: str = Form(..., description="Name for the submission"), + description: str | None = Form(None, description="Optional description"), +) -> APIResponse[AssessmentSubmissionResponse]: + """Upload a submission file (any CSV/Excel file, no column requirements).""" file_content, file_ext = await validate_dataset_file(file) - dataset = upload_assessment_dataset( + submission = upload_submission( session=session, file_content=file_content, file_ext=file_ext, - dataset_name=dataset_name, + submission_name=dataset_name, description=description, organization_id=auth_context.organization_.id, project_id=auth_context.project_.id, ) - return APIResponse.success_response(data=_dataset_to_response(dataset)) + return APIResponse.success_response(data=_submission_to_response(submission)) @router.get( "/datasets", description=load_description("assessment/list_datasets.md"), - response_model=APIResponse[list[AssessmentDatasetResponse]], + response_model=APIResponse[list[AssessmentSubmissionResponse]], dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], ) def list_datasets( session: SessionDep, auth_context: AuthContextDep, limit: int = Query( - default=50, ge=1, le=100, description="Maximum number of datasets to return" + default=50, ge=1, le=100, description="Maximum number of records to return" ), - offset: int = Query(default=0, ge=0, description="Number of datasets to skip"), -) -> APIResponse[list[AssessmentDatasetResponse]]: - """List assessment datasets.""" - datasets = list_assessment_datasets( + offset: int = Query(default=0, ge=0, description="Number of records to skip"), +) -> APIResponse[list[AssessmentSubmissionResponse]]: + """List uploaded submission files.""" + submissions = list_submissions( session=session, organization_id=auth_context.organization_.id, project_id=auth_context.project_.id, @@ -103,18 +97,18 @@ def list_datasets( ) return APIResponse.success_response( - data=[_dataset_to_response(dataset) for dataset in datasets] + data=[_submission_to_response(submission) for submission in submissions] ) @router.get( "/datasets/{dataset_id}", description=load_description("assessment/get_dataset.md"), - response_model=APIResponse[AssessmentDatasetResponse], + response_model=APIResponse[AssessmentSubmissionResponse], dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], ) def get_dataset( - dataset_id: int, + dataset_id: UUID, session: SessionDep, auth_context: AuthContextDep, include_signed_url: bool = Query( @@ -132,31 +126,31 @@ def get_dataset( ), ), ] = None, -) -> APIResponse[AssessmentDatasetResponse]: - """Get a specific assessment dataset.""" - dataset = get_assessment_dataset_by_id( +) -> APIResponse[AssessmentSubmissionResponse]: + """Get one uploaded submission file.""" + submission = get_submission_by_id( session=session, - dataset_id=dataset_id, + submission_id=dataset_id, organization_id=auth_context.organization_.id, project_id=auth_context.project_.id, ) signed_url = None - if include_signed_url and dataset.object_store_url: + if include_signed_url and submission.object_store_url: storage = get_cloud_storage( session=session, project_id=auth_context.project_.id ) - signed_url = storage.get_signed_url(dataset.object_store_url) + signed_url = storage.get_signed_url(submission.object_store_url) - preview: AssessmentDatasetPreview | None = None + preview: AssessmentSubmissionPreview | None = None if limit_rows is not None: - headers, rows = preview_assessment_dataset( + headers, rows = preview_submission( session=session, - dataset=dataset, + submission=submission, project_id=auth_context.project_.id, limit=limit_rows, ) - preview = AssessmentDatasetPreview( + preview = AssessmentSubmissionPreview( headers=headers, rows=rows, returned_rows=len(rows), @@ -164,7 +158,7 @@ def get_dataset( ) return APIResponse.success_response( - data=_dataset_to_response(dataset, signed_url=signed_url, preview=preview) + data=_submission_to_response(submission, signed_url=signed_url, preview=preview) ) @@ -175,28 +169,28 @@ def get_dataset( dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], ) def delete_dataset( - dataset_id: int, + dataset_id: UUID, session: SessionDep, auth_context: AuthContextDep, ) -> APIResponse[dict]: - """Delete an assessment dataset.""" - dataset = get_assessment_dataset_by_id( + """Delete an uploaded submission file.""" + submission = get_submission_by_id( session=session, - dataset_id=dataset_id, + submission_id=dataset_id, organization_id=auth_context.organization_.id, project_id=auth_context.project_.id, ) - dataset_name = dataset.name - error = delete_assessment_dataset(session=session, dataset=dataset) + submission_name = submission.name + error = delete_submission(session=session, submission=submission) if error: raise HTTPException(status_code=400, detail=error) return APIResponse.success_response( data={ "message": ( - f"Successfully deleted dataset '{dataset_name}' (id={dataset_id})" + f"Successfully deleted submission '{submission_name}' (id={dataset_id})" ), - "dataset_id": dataset_id, + "submission_id": str(dataset_id), } ) diff --git a/backend/app/api/routes/assessment/runs.py b/backend/app/api/routes/assessment/runs.py index 473b5dad9..a1c6a7332 100644 --- a/backend/app/api/routes/assessment/runs.py +++ b/backend/app/api/routes/assessment/runs.py @@ -1,6 +1,6 @@ """Assessment run endpoints — one row per config-run inside a parent assessment (LEGACY RUN pipeline). -Serves dataset-based RUN assessments only. The new API-client BATCH path +Serves submission-based RUN assessments only. The new API-client BATCH path (`api.py`) delivers results by webhook and never surfaces here. """ @@ -30,7 +30,7 @@ AssessmentRunPublic, AssessmentRunResponse, ) -from app.models.evaluation import EvaluationDataset +from app.models.assessment import AssessmentSubmission from app.services.assessment.service import ( resume_assessment_run as resume_run, ) @@ -58,7 +58,7 @@ def _build_run_public( session: SessionDep, run: AssessmentRun, ) -> AssessmentRunPublic: - """Build AssessmentRunPublic with parent-derived experiment/dataset info.""" + """Build AssessmentRunPublic with parent-derived experiment/submission info.""" parent = session.get(Assessment, run.assessment_id) if parent is None: logger.warning( @@ -66,13 +66,15 @@ def _build_run_public( run.assessment_id, run.id, ) - dataset = session.get(EvaluationDataset, parent.dataset_id) if parent else None + submission = ( + session.get(AssessmentSubmission, parent.submission_id) if parent else None + ) return AssessmentRunPublic( id=run.id, assessment_id=run.assessment_id, experiment_name=parent.experiment_name if parent else None, - dataset_id=parent.dataset_id if parent else None, - dataset_name=dataset.name if dataset else None, + submission_id=parent.submission_id if parent else None, + submission_name=submission.name if submission else None, config_id=run.config_id, config_version=run.config_version, status=run.status, @@ -104,9 +106,9 @@ def create_assessment_runs( ) -> APIResponse[AssessmentRunResponse]: """Submit an assessment and create one child run per config.""" logger.info( - "[create_assessment_runs] Assessment run submission | experiment=%s | dataset_id=%s | configs=%s", + "[create_assessment_runs] Assessment run submission | experiment=%s | submission_id=%s | configs=%s", request.experiment_name, - request.dataset_id, + request.submission_id, len(request.configs), ) diff --git a/backend/app/core/batch/operations.py b/backend/app/core/batch/operations.py index bcd51866b..d8db4151b 100644 --- a/backend/app/core/batch/operations.py +++ b/backend/app/core/batch/operations.py @@ -118,12 +118,12 @@ def process_completed_batch( provider: BatchProvider, batch_job: BatchJob, upload_to_object_store: bool = True, + subdirectory: str | None = None, ) -> tuple[list[dict[str, Any]], str | None]: - """ - Process a completed batch: download results and optionally upload to object store. + """Download a completed batch's results and optionally store them. - Returns: - Tuple of (results, object_store_url) + ``subdirectory`` overrides the default ``/batch-`` prefix. + Returns ``(results, object_store_url)``. """ logger.info(f"[process_completed_batch] Processing | id={batch_job.id}") @@ -134,7 +134,10 @@ def process_completed_batch( if upload_to_object_store: try: object_store_url = upload_batch_results_to_object_store( - session=session, batch_job=batch_job, results=results + session=session, + batch_job=batch_job, + results=results, + subdirectory=subdirectory, ) logger.info( f"[process_completed_batch] Uploaded to object store | {object_store_url}" @@ -160,9 +163,12 @@ def process_completed_batch( def upload_batch_results_to_object_store( - session: Session, batch_job: BatchJob, results: list[dict[str, Any]] + session: Session, + batch_job: BatchJob, + results: list[dict[str, Any]], + subdirectory: str | None = None, ) -> str | None: - """Upload batch results to object store.""" + """Upload batch results to object store; ``subdirectory`` overrides the default prefix.""" logger.info( f"[upload_batch_results_to_object_store] Uploading | batch_job_id={batch_job.id}" ) @@ -170,7 +176,7 @@ def upload_batch_results_to_object_store( try: storage = get_cloud_storage(session=session, project_id=batch_job.project_id) - subdirectory = f"{batch_job.job_type}/batch-{batch_job.id}" + subdirectory = subdirectory or f"{batch_job.job_type}/batch-{batch_job.id}" filename = "results.jsonl" object_store_url = shared_upload_jsonl( diff --git a/backend/app/core/batch/polling.py b/backend/app/core/batch/polling.py index c364aeb3d..27d8e45a0 100644 --- a/backend/app/core/batch/polling.py +++ b/backend/app/core/batch/polling.py @@ -11,6 +11,30 @@ logger = logging.getLogger(__name__) +# Status-result key -> batch_job column; the provider's transient `error_file_id` is ours. +_STATUS_RESULT_TO_COLUMN = { + "provider_status": "provider_status", + "provider_output_file_id": "provider_output_file_id", + "error_file_id": "provider_error_file_id", + "error_message": "error_message", +} + + +def _changed_columns( + status_result: dict[str, Any], batch_job: BatchJob +) -> dict[str, str]: + """Columns whose polled value differs from the row's. + + A field the provider omits (or reports empty) is left alone rather than nulled — + OpenAI only fills the file ids once the batch reaches a terminal state. + """ + changed: dict[str, str] = {} + for result_key, column in _STATUS_RESULT_TO_COLUMN.items(): + value = status_result.get(result_key) + if value and value != getattr(batch_job, column): + changed[column] = value + return changed + def poll_batch_status( session: Session, provider: BatchProvider, batch_job: BatchJob @@ -24,26 +48,20 @@ def poll_batch_status( try: status_result = provider.get_batch_status(batch_job.provider_batch_id) - provider_status = status_result["provider_status"] - if provider_status != batch_job.provider_status: - update_data = {"provider_status": provider_status} - - if status_result.get("provider_output_file_id"): - update_data["provider_output_file_id"] = status_result[ - "provider_output_file_id" - ] - - if status_result.get("error_message"): - update_data["error_message"] = status_result["error_message"] - - batch_job_update = BatchJobUpdate(**update_data) + # Per field, not gated on a status flip: an error file landing mid-status was dropped. + changed = _changed_columns(status_result, batch_job) + if changed: + previous_status = batch_job.provider_status batch_job = update_batch_job( - session=session, batch_job=batch_job, batch_job_update=batch_job_update + session=session, + batch_job=batch_job, + batch_job_update=BatchJobUpdate.model_validate(changed), ) logger.info( f"[poll_batch_status] Updated | id={batch_job.id} | " - f"{batch_job.provider_status} -> {provider_status}" + f"fields={sorted(changed)} | " + f"{previous_status} -> {batch_job.provider_status}" ) return status_result diff --git a/backend/app/crud/assessment/__init__.py b/backend/app/crud/assessment/__init__.py index a5cbb23b9..15a1f0714 100644 --- a/backend/app/crud/assessment/__init__.py +++ b/backend/app/crud/assessment/__init__.py @@ -21,11 +21,12 @@ update_assessment_run_status, update_run_post_processing_config, ) -from app.crud.assessment.dataset import ( - create_assessment_dataset, - delete_assessment_dataset, - get_assessment_dataset_by_id, - list_assessment_datasets, +from app.crud.assessment.submission import ( + create_submission, + delete_submission, + get_submission_by_id, + get_submission_by_name, + list_submissions, ) from app.models.assessment import AssessmentRunCounts, AssessmentRunStat @@ -35,18 +36,19 @@ "api", "build_run_stats", "compute_run_counts", - "create_assessment_dataset", + "create_submission", "create_assessment", "create_assessment_run", - "delete_assessment_dataset", + "delete_submission", "derive_aggregate_error", "derive_assessment_status", "get_assessment_by_id", - "get_assessment_dataset_by_id", + "get_submission_by_id", + "get_submission_by_name", "get_assessment_run_by_id", "get_assessment_runs_for_assessment", "list_assessment_runs", - "list_assessment_datasets", + "list_submissions", "list_assessments", "recompute_assessment_status", "update_assessment_run_prefilter_stats", diff --git a/backend/app/crud/assessment/api.py b/backend/app/crud/assessment/api.py index 33271ce24..feebf660d 100644 --- a/backend/app/crud/assessment/api.py +++ b/backend/app/crud/assessment/api.py @@ -2,15 +2,18 @@ Kept separate from the legacy RUN-pipeline crud (core/cron/processing/batch): writes only the new method-based columns and leaves the RUN-only `execution` -and `dataset_id` fields NULL. +and `submission_id` fields NULL. """ import logging from typing import Any, TypeVar, cast from uuid import UUID +from sqlalchemy import cast as sa_cast +from sqlalchemy import update +from sqlalchemy.dialects.postgresql import JSONB from sqlalchemy.orm.attributes import flag_modified -from sqlmodel import Session, select +from sqlmodel import Session, col, select from app.core.util import now from app.models.assessment import ( @@ -31,13 +34,15 @@ def create_assessment( *, session: Session, method: AssessmentMethod, - input: dict[str, Any], + input: dict[str, Any] | None, organization_id: int, project_id: int, + submission_id: UUID | None = None, ) -> Assessment: assessment = Assessment( method=method, input=input, + submission_id=submission_id, status=AssessmentStatus.PENDING, organization_id=organization_id, project_id=project_id, @@ -66,6 +71,21 @@ def set_assessment_job( return assessment +def set_submission_input( + *, session: Session, assessment: Assessment, url: str +) -> Assessment: + """Point the assessment at its stored submission rows.""" + assessment.submission_input = url + assessment.updated_at = now() + session.add(assessment) + session.commit() + session.refresh(assessment) + logger.info( + f"[set_submission_input] Linked submission | assessment_id: {assessment.id} | url: {url}" + ) + return assessment + + def create_execution( *, session: Session, @@ -129,6 +149,33 @@ def save_execution_state( return execution +def set_result_files( + *, session: Session, assessment: Assessment, files: dict[str, dict[str, Any]] +) -> Assessment: + """Shallow-merge ``files`` into ``assessment.result_files``, one record per file kind. + + The merge is server-side (``||``, right-hand side wins per key) because two drivers + can touch this row within the same second; a read-modify-write would drop the loser's + kinds instead of keeping both. + """ + statement = ( + update(Assessment) + .where(col(Assessment.id) == assessment.id) + .values( + result_files=col(Assessment.result_files).op("||")(sa_cast(files, JSONB)), + updated_at=now(), + ) + ) + session.exec(statement) + session.commit() + session.refresh(assessment) + logger.info( + f"[set_result_files] Merged result files | assessment_id: {assessment.id} | " + f"kinds: {sorted(files)}" + ) + return assessment + + def update_status( *, session: Session, obj: StatusModel, status: AssessmentStatus ) -> StatusModel: diff --git a/backend/app/crud/assessment/batch.py b/backend/app/crud/assessment/batch.py index 1c4464525..14b39e959 100644 --- a/backend/app/crud/assessment/batch.py +++ b/backend/app/crud/assessment/batch.py @@ -1,6 +1,6 @@ """Assessment batch JSONL construction and submission. -Builds provider-specific JSONL files from dataset rows + config, +Builds provider-specific JSONL files from submission rows + config, then submits them via the core batch infrastructure. """ @@ -26,9 +26,9 @@ Assessment, AssessmentAttachment, AssessmentRun, + AssessmentSubmission, ) from app.models.batch_job import BatchJob, BatchJobType -from app.models.evaluation import EvaluationDataset from app.models.llm.constants import DEFAULT_ASSESSMENT_BATCH_MAX_TOKENS from app.models.llm.request import ConfigBlob from app.services.assessment.mappers import ( @@ -37,6 +37,7 @@ map_kaapi_to_openai_params, normalize_llm_text, ) +from app.services.assessment.submission import file_extension_of from app.services.assessment.utils.attachments import ( attachment_type_for_row, build_anthropic_attachment_parts, @@ -51,28 +52,23 @@ logger = logging.getLogger(__name__) -def _load_dataset_rows( +def load_submission_file_rows( + *, session: Session, - dataset: EvaluationDataset, + submission: AssessmentSubmission, ) -> list[dict[str, str]]: - """Load dataset rows from object store. + """Read an uploaded submission file into row dicts keyed by column name.""" + if not submission.object_store_url: + raise ValueError(f"Submission {submission.id} has no object_store_url") - Returns a list of dicts (one per row) with column-name keys. - """ - if not dataset.object_store_url: - raise ValueError(f"Dataset {dataset.id} has no object_store_url") - - storage = get_cloud_storage(session=session, project_id=dataset.project_id) - - # Download the file content via stream() - body = storage.stream(dataset.object_store_url) - file_content = body.read() + storage = get_cloud_storage(session=session, project_id=submission.project_id) + file_content = storage.stream(submission.object_store_url).read() if not file_content: - raise ValueError(f"Failed to download dataset from {dataset.object_store_url}") - - metadata = dataset.dataset_metadata or {} - file_ext = metadata.get("file_extension", ".csv") + raise ValueError( + f"Failed to download submission from {submission.object_store_url}" + ) + file_ext = file_extension_of(submission.object_store_url) if file_ext == ".xls": raise ValueError( "Legacy Excel format (.xls) is not supported. Please upload .xlsx or .csv." @@ -133,7 +129,7 @@ def _parse_excel_rows(content: bytes) -> list[dict[str, str]]: logger.warning( "[_parse_excel_rows] Failed to parse XLSX rows | %s", e, exc_info=True ) - raise ValueError("Failed to parse XLSX dataset rows") from e + raise ValueError("Failed to parse XLSX submission rows") from e finally: if wb is not None: wb.close() @@ -173,7 +169,7 @@ def build_openai_jsonl( openai_params: dict, row_indices: list[int] | None = None, ) -> list[dict[str, Any]]: - """Build OpenAI batch JSONL data from dataset rows. + """Build OpenAI batch JSONL data from submission rows. Each line follows the OpenAI batch format: { @@ -239,7 +235,7 @@ def build_google_jsonl( google_params: dict, row_indices: list[int] | None = None, ) -> list[dict[str, Any]]: - """Build Google (Gemini) batch JSONL data from dataset rows. + """Build Google (Gemini) batch JSONL data from submission rows. Each line follows the Gemini batch format: { @@ -318,7 +314,7 @@ def build_anthropic_jsonl( anthropic_params: dict, row_indices: list[int] | None = None, ) -> list[dict[str, Any]]: - """Build Anthropic batch request data from dataset rows. + """Build Anthropic batch request data from submission rows. Each line follows the Anthropic Message Batches format: { @@ -368,7 +364,7 @@ def submit_assessment_batch( session: Session, run: AssessmentRun, assessment: Assessment, - dataset: EvaluationDataset, + submission: AssessmentSubmission, config_blob: ConfigBlob, assessment_input: dict[str, Any], organization_id: int, @@ -378,20 +374,10 @@ def submit_assessment_batch( ) -> BatchJob: """Build JSONL and submit a batch for one assessment run. - Args: - session: Database session - run: The AssessmentRun to process - dataset: The dataset to read rows from - config_blob: Resolved configuration blob - assessment_input: Parent InputBinding (prompt, text_columns, attachments) - organization_id: Organization ID - project_id: Project ID - - Returns: - Created BatchJob record + Rows come from ``preloaded_rows`` when the caller already filtered them + (post-prefilter), else from the submission file. """ - # `assessment_input` is the parent InputBinding (prompt/text_columns/attachments); - # system_instruction / output_schema now live in the resolved config blob. + # `assessment_input` is the parent InputBinding; the rest lives in the config blob. text_columns = assessment_input.get("text_columns", []) prompt_template = assessment_input.get("prompt") attachments_raw = assessment_input.get("attachments", []) @@ -401,9 +387,9 @@ def submit_assessment_batch( if preloaded_rows is not None: rows = preloaded_rows else: - rows = _load_dataset_rows(session, dataset) + rows = load_submission_file_rows(session=session, submission=submission) if not rows: - raise ValueError(f"Dataset {dataset.id} has no rows") + raise ValueError(f"Submission {submission.id} has no rows") logger.info( "[submit_assessment_batch] Building JSONL | run_id=%s | rows=%s | provider=%s", diff --git a/backend/app/crud/assessment/core.py b/backend/app/crud/assessment/core.py index 450a74ed3..c51820be8 100644 --- a/backend/app/crud/assessment/core.py +++ b/backend/app/crud/assessment/core.py @@ -35,7 +35,7 @@ def _write_exec(run: AssessmentRun, **values: Any) -> None: def create_assessment( session: Session, experiment_name: str, - dataset_id: int, + submission_id: UUID, organization_id: int, project_id: int, input_binding: dict[str, Any] | None = None, @@ -48,7 +48,7 @@ def create_assessment( assessment = Assessment( experiment_name=experiment_name, method=AssessmentMethod.RUN, - dataset_id=dataset_id, + submission_id=submission_id, input=input_binding, status=AssessmentStatus.PENDING, organization_id=organization_id, diff --git a/backend/app/crud/assessment/cron.py b/backend/app/crud/assessment/cron.py index 342a4e247..3d874914b 100644 --- a/backend/app/crud/assessment/cron.py +++ b/backend/app/crud/assessment/cron.py @@ -18,6 +18,7 @@ ) from app.models.assessment import ( Assessment, + AssessmentMethod, AssessmentRun, AssessmentStatus, StageStatus, @@ -25,6 +26,9 @@ logger = logging.getLogger(__name__) +# Programming errors: a retry just re-runs the same broken code, so fail the run instead. +DETERMINISTIC_ERRORS = (ValueError, AttributeError, TypeError, KeyError, IndexError) + def _log_config_progress( result: dict[str, Any], run: AssessmentRun, assessment: Assessment @@ -52,8 +56,13 @@ def _log_config_progress( async def poll_all_pending_assessment_evaluations( session: Session, ) -> dict[str, Any]: - """Poll all non-terminal parent assessments and their active child runs.""" + """Poll all non-terminal RUN parent assessments and their active child runs. + + RUN only: the BATCH API path is driven by its own Celery self-re-enqueue and stores a + differently shaped ``execution`` bag that this poller corrupts. + """ statement = select(Assessment).where( + Assessment.method == AssessmentMethod.RUN, Assessment.status.in_((AssessmentStatus.PENDING, AssessmentStatus.PROCESSING)), ) pending_assessments = list(session.exec(statement).all()) @@ -127,7 +136,7 @@ async def poll_all_pending_assessment_evaluations( else: still_processing += 1 - except ValueError as e: + except DETERMINISTIC_ERRORS as e: session.rollback() message = format_assessment_failure_message(e) logger.error( diff --git a/backend/app/crud/assessment/dataset.py b/backend/app/crud/assessment/dataset.py deleted file mode 100644 index d195ae8e5..000000000 --- a/backend/app/crud/assessment/dataset.py +++ /dev/null @@ -1,163 +0,0 @@ -"""CRUD operations for assessment datasets.""" - -import logging -from typing import Any - -from fastapi import HTTPException -from sqlalchemy.exc import IntegrityError -from sqlmodel import Session, select - -from app.core.util import now -from app.models.assessment import Assessment -from app.models.evaluation import EvaluationDataset -from app.models.stt_evaluation import EvaluationType - -logger = logging.getLogger(__name__) - - -def create_assessment_dataset( - *, - session: Session, - name: str, - dataset_metadata: dict[str, Any], - organization_id: int, - project_id: int, - description: str | None = None, - object_store_url: str | None = None, - langfuse_dataset_id: str | None = None, -) -> EvaluationDataset: - """Create an assessment dataset backed by the shared evaluation_dataset table.""" - dataset = EvaluationDataset( - name=name, - description=description, - type=EvaluationType.ASSESSMENT.value, - dataset_metadata=dataset_metadata, - object_store_url=object_store_url, - langfuse_dataset_id=langfuse_dataset_id, - organization_id=organization_id, - project_id=project_id, - inserted_at=now(), - updated_at=now(), - ) - - try: - session.add(dataset) - session.commit() - session.refresh(dataset) - except IntegrityError as e: - session.rollback() - logger.warning( - "[create_assessment_dataset] Dataset name already exists | " - "name=%s | org_id=%s | project_id=%s", - name, - organization_id, - project_id, - exc_info=True, - ) - raise HTTPException( - status_code=409, - detail=( - f"Dataset with name '{name}' already exists in this " - "organization and project. Please choose a different name." - ), - ) from e - except Exception as e: - session.rollback() - logger.error( - "[create_assessment_dataset] Failed to create dataset | name=%s", - name, - exc_info=True, - ) - raise HTTPException( - status_code=500, - detail=f"Failed to save assessment dataset metadata", - ) from e - - logger.info( - "[create_assessment_dataset] Created assessment dataset | " - "id=%s | name=%s | org_id=%s | project_id=%s", - dataset.id, - name, - organization_id, - project_id, - ) - return dataset - - -def get_assessment_dataset_by_id( - *, - session: Session, - dataset_id: int, - organization_id: int, - project_id: int, -) -> EvaluationDataset: - """Fetch an assessment dataset by ID, scoped to organization and project.""" - statement = ( - select(EvaluationDataset) - .where(EvaluationDataset.id == dataset_id) - .where(EvaluationDataset.organization_id == organization_id) - .where(EvaluationDataset.project_id == project_id) - .where(EvaluationDataset.type == EvaluationType.ASSESSMENT.value) - ) - dataset = session.exec(statement).first() - if not dataset: - raise HTTPException( - status_code=404, - detail=f"Dataset {dataset_id} not found or not accessible", - ) - return dataset - - -def list_assessment_datasets( - *, - session: Session, - organization_id: int, - project_id: int, - limit: int = 50, - offset: int = 0, -) -> list[EvaluationDataset]: - """List assessment datasets for an organization and project.""" - statement = ( - select(EvaluationDataset) - .where(EvaluationDataset.organization_id == organization_id) - .where(EvaluationDataset.project_id == project_id) - .where(EvaluationDataset.type == EvaluationType.ASSESSMENT.value) - .order_by(EvaluationDataset.inserted_at.desc()) - .limit(limit) - .offset(offset) - ) - return list(session.exec(statement).all()) - - -def delete_assessment_dataset( - *, session: Session, dataset: EvaluationDataset -) -> str | None: - """Delete an unused assessment dataset.""" - statement = select(Assessment).where(Assessment.dataset_id == dataset.id) - assessments = session.exec(statement).all() - if assessments: - return ( - f"Cannot delete dataset {dataset.id}: it is being used by " - f"{len(assessments)} assessment(s). Please delete the assessments first." - ) - - try: - dataset_id = dataset.id - dataset_name = dataset.name - session.delete(dataset) - session.commit() - except Exception as e: - session.rollback() - logger.error( - "[delete_assessment_dataset] Failed to delete dataset | dataset_id=%s", - dataset.id, - exc_info=True, - ) - return f"Failed to delete dataset: {e}" - - logger.info( - "[delete_assessment_dataset] Deleted assessment dataset | id=%s | name=%s", - dataset_id, - dataset_name, - ) - return None diff --git a/backend/app/crud/assessment/submission.py b/backend/app/crud/assessment/submission.py new file mode 100644 index 000000000..28d13f30e --- /dev/null +++ b/backend/app/crud/assessment/submission.py @@ -0,0 +1,166 @@ +"""CRUD operations for uploaded assessment submissions.""" + +import logging +from uuid import UUID + +from fastapi import HTTPException +from sqlalchemy.exc import IntegrityError +from sqlmodel import Session, select + +from app.core.util import now +from app.models.assessment import Assessment, AssessmentSubmission + +logger = logging.getLogger(__name__) + + +def get_submission_by_name( + *, session: Session, name: str, organization_id: int, project_id: int +) -> AssessmentSubmission | None: + """Fetch a submission by the columns the unique constraint covers.""" + statement = ( + select(AssessmentSubmission) + .where(AssessmentSubmission.name == name) + .where(AssessmentSubmission.organization_id == organization_id) + .where(AssessmentSubmission.project_id == project_id) + ) + return session.exec(statement).first() + + +def create_submission( + *, + session: Session, + name: str, + object_store_url: str, + total_items: int, + organization_id: int, + project_id: int, + description: str | None = None, +) -> AssessmentSubmission: + """Record an uploaded submission file.""" + submission = AssessmentSubmission( + name=name, + description=description, + object_store_url=object_store_url, + total_items=total_items, + organization_id=organization_id, + project_id=project_id, + inserted_at=now(), + updated_at=now(), + ) + + try: + session.add(submission) + session.commit() + session.refresh(submission) + except IntegrityError as e: + # Backstop for two concurrent uploads; the caller already checks the name. + session.rollback() + logger.warning( + "[create_submission] Name already exists | name=%s | org_id=%s | project_id=%s", + name, + organization_id, + project_id, + exc_info=True, + ) + raise HTTPException( + status_code=409, + detail=( + f"Submission with name '{name}' already exists in this " + "organization and project. Please choose a different name." + ), + ) from e + except Exception as e: + session.rollback() + logger.error( + "[create_submission] Failed to create submission | name=%s", + name, + exc_info=True, + ) + raise HTTPException( + status_code=500, detail="Failed to save the submission metadata." + ) from e + + logger.info( + "[create_submission] Created | id=%s | name=%s | rows=%s | org_id=%s | project_id=%s", + submission.id, + name, + total_items, + organization_id, + project_id, + ) + return submission + + +def get_submission_by_id( + *, + session: Session, + submission_id: UUID, + organization_id: int, + project_id: int, +) -> AssessmentSubmission: + """Fetch a submission by id, scoped to organization and project.""" + statement = ( + select(AssessmentSubmission) + .where(AssessmentSubmission.id == submission_id) + .where(AssessmentSubmission.organization_id == organization_id) + .where(AssessmentSubmission.project_id == project_id) + ) + submission = session.exec(statement).first() + if not submission: + raise HTTPException( + status_code=404, + detail=f"Submission {submission_id} not found or not accessible", + ) + return submission + + +def list_submissions( + *, + session: Session, + organization_id: int, + project_id: int, + limit: int = 50, + offset: int = 0, +) -> list[AssessmentSubmission]: + """List submissions for an organization and project, newest first.""" + statement = ( + select(AssessmentSubmission) + .where(AssessmentSubmission.organization_id == organization_id) + .where(AssessmentSubmission.project_id == project_id) + .order_by(AssessmentSubmission.inserted_at.desc()) + .limit(limit) + .offset(offset) + ) + return list(session.exec(statement).all()) + + +def delete_submission( + *, session: Session, submission: AssessmentSubmission +) -> str | None: + """Delete a submission no assessment references. Returns a reason when refused.""" + statement = select(Assessment).where(Assessment.submission_id == submission.id) + assessments = session.exec(statement).all() + if assessments: + return ( + f"Cannot delete submission {submission.id}: it is being used by " + f"{len(assessments)} assessment(s). Please delete the assessments first." + ) + + submission_id = submission.id + submission_name = submission.name + try: + session.delete(submission) + session.commit() + except Exception as e: + session.rollback() + logger.error( + "[delete_submission] Failed to delete | submission_id=%s", + submission.id, + exc_info=True, + ) + return f"Failed to delete submission: {e}" + + logger.info( + "[delete_submission] Deleted | id=%s | name=%s", submission_id, submission_name + ) + return None diff --git a/backend/app/models/assessment/__init__.py b/backend/app/models/assessment/__init__.py index dbc581713..b6dbbb837 100644 --- a/backend/app/models/assessment/__init__.py +++ b/backend/app/models/assessment/__init__.py @@ -1,19 +1,14 @@ """Assessment models package. -Split across two files so the API-client surface is separate from the legacy UI one: - - ``assessment.py`` — the shared DB tables/enums + the legacy Assessment Run UI models. - - ``assessment_api.py`` — the API-client request/response models for ``/assessments``. - -Everything is re-exported here, so ``from app.models.assessment import X`` resolves a -symbol from either file, unchanged for every existing importer. +Split by surface: ``assessment.py`` (shared tables/enums + legacy RUN UI), +``assessment_api.py`` (``/assessments`` request/response), ``submission.py`` (uploaded +submission files). All re-exported here, so ``from app.models.assessment import X`` works. """ from app.models.assessment.assessment import ( Assessment, AssessmentAttachment, AssessmentConfigRef, - AssessmentDatasetPreview, - AssessmentDatasetResponse, AssessmentExecutionPublic, AssessmentExportRow, AssessmentMethod, @@ -54,6 +49,11 @@ Verdict, derive_method, ) +from app.models.assessment.submission import ( + AssessmentSubmission, + AssessmentSubmissionPreview, + AssessmentSubmissionResponse, +) __all__ = [ # shared tables + enums @@ -79,8 +79,10 @@ "AssessmentRunPublic", "AssessmentRunResponse", "AssessmentRunOverview", - "AssessmentDatasetPreview", - "AssessmentDatasetResponse", + # uploaded submission files + "AssessmentSubmission", + "AssessmentSubmissionPreview", + "AssessmentSubmissionResponse", # API-client surface "Attachment", "ResponseInput", diff --git a/backend/app/models/assessment/assessment.py b/backend/app/models/assessment/assessment.py index 3b4d9d249..b8cc95742 100644 --- a/backend/app/models/assessment/assessment.py +++ b/backend/app/models/assessment/assessment.py @@ -123,7 +123,8 @@ class RunExecution(BaseModel): stage: Stage | None = None stage_status: StageStatus | None = None - pipeline: dict[str, Any] | None = None + # Two writers, one column: RUN stores {"stages": [...]}, the BATCH API a bare list. + pipeline: dict[str, Any] | list[dict[str, str]] | None = None stage_batches: dict[str, int] | None = None prefilter_total_rows: int | None = None prefilter_total_passed: int | None = None @@ -185,17 +186,43 @@ class Assessment(SQLModel, table=True): sa_column=Column( JSONB, nullable=True, - comment="Method-shaped: ResponseInput (RESPONSE) / BatchInput (BATCH) / InputBinding (RUN)", + comment="Method-shaped: ResponseInput (RESPONSE) / InputBinding (RUN); NULL for API-client BATCH, which uses submission_input", ), ) - # NOTE: Legacy, this is for Assessment Run UI only. The new Assessment pipeline does not use this. - dataset_id: int | None = SQLField( + submission_input: str | None = SQLField( default=None, - foreign_key="evaluation_dataset.id", + sa_column_kwargs={ + "comment": ( + "Object-store url of the API-client BATCH submission rows " + "(submission.jsonl); the rows are never stored in this table" + ) + }, + ) + # NOT NULL '{}': an empty map is a real state, not the siblings' "not applicable". + result_files: dict[str, Any] = SQLField( + default_factory=dict, + sa_column=Column( + JSONB, + nullable=False, + server_default=text("'{}'::jsonb"), + comment=( + "Result-file kind (results / errors / _results) to " + "{object_store_url} for every provider batch dump held; raw s3:// in the " + "column, presigned per delivery in the BATCH callback" + ), + ), + ) + submission_id: UUID | None = SQLField( + default=None, + foreign_key="assessment_submission.id", nullable=True, + index=True, ondelete="SET NULL", sa_column_kwargs={ - "comment": "External dataset (RUN); binding lives in `input`" + "comment": ( + "Uploaded submission the rows came from; set by RUN and by a BATCH " + "submitted with `submission_doc_id`. NULL when BATCH sent rows inline" + ) }, ) @@ -333,7 +360,7 @@ class AssessmentExportRow(BaseModel): # NOTE: Legacy, this is for Assessment Run UI only. The new Assessment pipeline does not use this. class AssessmentRunCreate(BaseModel): experiment_name: str - dataset_id: int + submission_id: UUID input_binding: InputBinding configs: list[AssessmentConfigRef] = Field(min_length=1, max_length=4) post_processing_config: dict[str, Any] | None = None @@ -388,8 +415,8 @@ class AssessmentRunPublic(BaseModel): class AssessmentRunResponse(BaseModel): assessment_id: UUID experiment_name: str | None = None - dataset_id: int | None = None - dataset_name: str | None = None + submission_id: UUID | None = None + submission_name: str | None = None num_configs: int runs: list[AssessmentRunSummary] = [] @@ -399,8 +426,8 @@ class AssessmentRunOverview(BaseModel): id: UUID experiment_name: str | None = None status: AssessmentStatus - dataset_id: int | None = None - dataset_name: str | None = None + submission_id: UUID | None = None + submission_name: str | None = None input_binding: InputBinding | None = None counts: AssessmentRunCounts = AssessmentRunCounts() run_stats: list[AssessmentRunStat] = [] @@ -408,23 +435,3 @@ class AssessmentRunOverview(BaseModel): project_id: int inserted_at: datetime updated_at: datetime - - -# NOTE: Legacy, this is for Assessment Run UI only. The new Assessment pipeline does not use this. -class AssessmentDatasetPreview(BaseModel): - headers: list[str] - rows: list[list[str]] - returned_rows: int = 0 - truncated: bool = False - - -# NOTE: Legacy, this is for Assessment Run UI only. The new Assessment pipeline does not use this. -class AssessmentDatasetResponse(BaseModel): - dataset_id: int - dataset_name: str - description: str | None = None - total_items: int = 0 - file_extension: str | None = None - object_store_url: str | None = None - signed_url: str | None = None - preview: AssessmentDatasetPreview | None = None diff --git a/backend/app/models/assessment/assessment_api.py b/backend/app/models/assessment/assessment_api.py index 21b363222..ae0dac5a6 100644 --- a/backend/app/models/assessment/assessment_api.py +++ b/backend/app/models/assessment/assessment_api.py @@ -8,7 +8,7 @@ from typing import Annotated, Any, NotRequired, TypedDict from uuid import UUID -from pydantic import BaseModel, ConfigDict, Field, HttpUrl +from pydantic import BaseModel, ConfigDict, Field, HttpUrl, model_validator from sqlmodel import SQLModel from app.models.assessment.assessment import ( @@ -35,13 +35,31 @@ class ResponseInput(SQLModel): class BatchInput(SQLModel): - """BATCH input — a list of submission rows. The prompt template lives in the config.""" + """BATCH input — rows inline, or a pointer to an uploaded submission file. + + The two are mutually exclusive: exactly one must be given. The prompt template + lives in the config either way. + """ model_config = ConfigDict(extra="forbid") - data: list[Submission] = Field( - ..., min_length=1, description="Submission rows; one assessed item each" + data: list[Submission] | None = Field( + default=None, + min_length=1, + description="Submission rows; one assessed item each", ) + submission_doc_id: UUID | None = Field( + default=None, + description="Id of an uploaded submission file to read the rows from", + ) + + @model_validator(mode="after") + def _exactly_one_row_source(self) -> "BatchInput": + if (self.data is None) == (self.submission_doc_id is None): + raise ValueError( + "Provide exactly one of 'data' (inline rows) or 'submission_doc_id'." + ) + return self class Verdict(TypedDict): @@ -75,6 +93,8 @@ class BatchRunState(TypedDict): # raw_output_url is Optional, so the map value types must admit None. stage_batches: dict[str, int | None] # stage -> provider batch_job id stage_output_urls: dict[str, str | None] # stage -> raw result url + # stage -> {row_index -> error}, captured at parse time (raw dumps are too big here) + stage_errors: NotRequired[dict[str, dict[str, str]]] verdicts: dict[str, dict[str, Verdict]] # stage -> {item_idx -> verdict} counters: dict[str, dict[str, int]] # stage -> {total,passed,rejected} gate_passed: list[bool] # per-item still-eligible flag @@ -93,16 +113,16 @@ class BatchRunState(TypedDict): def derive_method( - input_: AssessmentInput | None, dataset_id: int | None + input_: AssessmentInput | None, submission_id: UUID | None ) -> AssessmentMethod: - """Infer method: ResponseInput ⇒ RESPONSE, BatchInput ⇒ BATCH, else dataset_id ⇒ RUN.""" + """Infer method: ResponseInput ⇒ RESPONSE, BatchInput ⇒ BATCH, else submission_id ⇒ RUN.""" if isinstance(input_, ResponseInput): return AssessmentMethod.RESPONSE if isinstance(input_, BatchInput): return AssessmentMethod.BATCH - if dataset_id is not None: + if submission_id is not None: return AssessmentMethod.RUN - raise ValueError("[derive_method] Provide inline `input` or `dataset_id`") + raise ValueError("[derive_method] Provide inline `input` or `submission_id`") class AssessmentCreate(BaseModel): diff --git a/backend/app/models/assessment/submission.py b/backend/app/models/assessment/submission.py new file mode 100644 index 000000000..acaf2a39b --- /dev/null +++ b/backend/app/models/assessment/submission.py @@ -0,0 +1,82 @@ +"""Uploaded submission files for the assessment domain. + +Its own table rather than a row in ``evaluation_dataset``: that table multiplexes four +surfaces behind a ``type`` column, carries eval-only fields, and its name uniqueness is +type-agnostic, so an eval dataset name blocks an assessment one. +""" + +from datetime import datetime +from uuid import UUID, uuid4 + +from pydantic import BaseModel +from sqlmodel import Field as SQLField +from sqlmodel import SQLModel, UniqueConstraint + +from app.core.util import now + + +class AssessmentSubmission(SQLModel, table=True): + """One uploaded submission file (CSV/XLSX) an assessment can be run against.""" + + __tablename__ = "assessment_submission" + __table_args__ = ( + UniqueConstraint( + "name", + "organization_id", + "project_id", + name="uq_assessment_submission_name_org_project", + ), + ) + + id: UUID = SQLField( + default_factory=uuid4, + primary_key=True, + sa_column_kwargs={"comment": "Unique identifier for the submission"}, + ) + name: str = SQLField( + index=True, + sa_column_kwargs={ + "comment": "Sanitized name; the object key is derived from it" + }, + ) + description: str | None = SQLField( + default=None, sa_column_kwargs={"comment": "Optional description"} + ) + object_store_url: str = SQLField( + sa_column_kwargs={ + "comment": "Object-store url of the uploaded file; its suffix gives the format" + } + ) + total_items: int = SQLField( + default=0, sa_column_kwargs={"comment": "Row count, excluding the header"} + ) + + organization_id: int = SQLField( + foreign_key="organization.id", nullable=False, ondelete="CASCADE" + ) + project_id: int = SQLField( + foreign_key="project.id", nullable=False, ondelete="CASCADE" + ) + inserted_at: datetime = SQLField(default_factory=now, nullable=False) + updated_at: datetime = SQLField(default_factory=now, nullable=False) + + +class AssessmentSubmissionPreview(BaseModel): + """First N rows of a submission file, for the upload confirmation screen.""" + + headers: list[str] + rows: list[list[str]] + returned_rows: int = 0 + truncated: bool = False + + +class AssessmentSubmissionResponse(BaseModel): + """API shape for a stored submission; ``signed_url`` is minted per request.""" + + submission_id: UUID + name: str + description: str | None = None + total_items: int = 0 + object_store_url: str | None = None + signed_url: str | None = None + preview: AssessmentSubmissionPreview | None = None diff --git a/backend/app/models/batch_job.py b/backend/app/models/batch_job.py index 426d44f59..92e5763c7 100644 --- a/backend/app/models/batch_job.py +++ b/backend/app/models/batch_job.py @@ -83,6 +83,16 @@ class BatchJob(SQLModel, table=True): description="Provider's output file ID", sa_column_kwargs={"comment": "Provider's output file ID"}, ) + provider_error_file_id: str | None = Field( + default=None, + description="Provider's error file ID (OpenAI only)", + sa_column_kwargs={ + "comment": ( + "Provider's error file ID (OpenAI only; Anthropic and Gemini report " + "per-item errors inline)" + ) + }, + ) # Provider status tracking provider_status: str | None = Field( @@ -171,6 +181,7 @@ class BatchJobUpdate(SQLModel): provider_batch_id: str | None = None provider_file_id: str | None = None provider_output_file_id: str | None = None + provider_error_file_id: str | None = None provider_status: str | None = None raw_output_url: str | None = None total_items: int | None = None @@ -187,6 +198,7 @@ class BatchJobPublic(SQLModel): provider_batch_id: str | None provider_file_id: str | None provider_output_file_id: str | None + provider_error_file_id: str | None provider_status: str | None raw_output_url: str | None total_items: int diff --git a/backend/app/models/llm/request.py b/backend/app/models/llm/request.py index c028a978a..19860ed34 100644 --- a/backend/app/models/llm/request.py +++ b/backend/app/models/llm/request.py @@ -82,6 +82,17 @@ class TextLLMParams(ParamSerialization, SQLModel): "Model-specific reasoning summary preference. " "Use null/None to disable." ), ) + thinking: dict[str, Any] | None = Field( + default=None, + description=( + "Anthropic adaptive-thinking container, forwarded to the provider as-is " + "(e.g. {'type': 'enabled', 'budget_tokens': 4096})" + ), + ) + thinking_level: Literal["low", "medium", "high"] | None = Field( + default=None, + description="Google thinking level for thinking-capable Gemini models", + ) temperature: float | None = Field( default=0.1, ge=0.0, diff --git a/backend/app/services/assessment/api/batch.py b/backend/app/services/assessment/api/batch.py index 79e464edd..5eef146d0 100644 --- a/backend/app/services/assessment/api/batch.py +++ b/backend/app/services/assessment/api/batch.py @@ -18,6 +18,7 @@ import logging from enum import StrEnum from typing import Any, cast +from uuid import UUID from sqlmodel import Session @@ -598,19 +599,44 @@ def _submit_stage( *, session: Session, execution: AssessmentRun, + assessment: Assessment, blob: AssessmentConfigBlob, - batch_input: BatchInput, bag: BatchRunState, stage: str, organization_id: int, project_id: int, ) -> bool: - """Build + submit the current stage's batch on its row subset. Returns success.""" + """Build + submit the current stage's batch on its row subset. Returns success. + + A stage already in flight is reported as submitted (no second provider batch): the + in-memory bag can predate a concurrent tick's write, so the row is re-read here. + The submission rows are fetched here, not per tick: only a submission needs them. + """ + session.refresh(execution) + persisted = cast(BatchRunState, execution.execution or {}) + in_flight_batch_id = (persisted.get("stage_batches") or {}).get(stage) + if ( + in_flight_batch_id is not None + and persisted.get("stage_status") == StageStatus.PROCESSING.value + ): + # Deliberately leaves `bag` unpersisted: the stored state is the fresher one. + logger.warning( + "[_submit_stage] Stage already submitted, skipping duplicate | " + "execution_id=%s | stage=%s | batch_job=%s", + execution.id, + stage, + in_flight_batch_id, + ) + return True + + from app.services.assessment.api.submission_store import load_submission_rows + kind = _stage_kind(bag["pipeline"], stage) input_columns = { name: col.model_dump(exclude_none=True) for name, col in blob.input_schema.items() } + batch_input = load_submission_rows(session=session, assessment=assessment) rows, text_columns, attachments = build_rows(batch_input, input_columns) subset = _row_subset(bag, stage, kind, len(rows)) @@ -661,7 +687,10 @@ def _submit_stage( def _poll_outcome( - session: Session, provider: BatchProvider, batch_job: BatchJob + session: Session, + provider: BatchProvider, + batch_job: BatchJob, + assessment_id: UUID, ) -> tuple[str, list[dict[str, Any]] | None]: """Poll a stage batch. Returns ('processing'|'completed'|'failed', results).""" status_result = poll_batch_status( @@ -679,8 +708,17 @@ def _poll_outcome( ): return "failed", None if batch_job.provider_output_file_id: + from app.services.assessment.api.result_files import ( + assessment_subdirectory, + ) + results, _ = process_completed_batch( - session=session, provider=provider, batch_job=batch_job + session=session, + provider=provider, + batch_job=batch_job, + subdirectory=( + f"{assessment_subdirectory(assessment_id)}/batch-{batch_job.id}" + ), ) return "completed", results return "processing", None # output not ready yet @@ -696,6 +734,7 @@ def _finalize( bag: BatchRunState, ) -> None: from app.services.assessment.api.callbacks import deliver + from app.services.assessment.api.result_files import finalize_result_files from app.services.assessment.api.results import build_result bag["stage"] = ApiStage.ASSESSMENT.value @@ -721,13 +760,20 @@ def _finalize( errors, ) + # Before the callback check: durability must not depend on a callback existing. + finalize_result_files( + session=session, execution=execution, assessment=assessment, bag=bag + ) + callback_url = bag.get("callback_url") if callback_url: deliver( + session=session, assessment=assessment, result=result, callback_url=callback_url, request_metadata=bag.get("request_metadata"), + failure_message=None, ) @@ -739,6 +785,7 @@ def _fail( message: str, ) -> None: from app.services.assessment.api.callbacks import deliver + from app.services.assessment.api.result_files import finalize_result_files from app.services.assessment.api.results import build_result bag["stage_status"] = StageStatus.FAILED.value @@ -751,14 +798,25 @@ def _fail( "[_fail] Execution failed | execution_id=%s | message=%s", execution.id, message ) + # Before the callback check: durability must not depend on a callback existing. + finalize_result_files( + session=session, + execution=execution, + assessment=assessment, + bag=bag, + failure_message=message, + ) + callback_url = bag.get("callback_url") if callback_url: result = build_result(session=session, assessment=assessment) deliver( + session=session, assessment=assessment, result=result, callback_url=callback_url, request_metadata=bag.get("request_metadata"), + failure_message=message, ) @@ -768,7 +826,6 @@ def _advance_or_finalize( execution: AssessmentRun, assessment: Assessment, blob: AssessmentConfigBlob, - batch_input: BatchInput, bag: BatchRunState, stage: str, organization_id: int, @@ -780,6 +837,8 @@ def _advance_or_finalize( (``_submit_stage`` returns False); it is treated as completed and skipped, recursing so a chain of empty stages still terminates at ``_finalize``. """ + from app.services.assessment.api.submission_store import SubmissionUnavailableError + nxt = next_stage(bag["pipeline"], stage) if nxt is None: _finalize(session, execution, assessment, bag) @@ -790,13 +849,23 @@ def _advance_or_finalize( submitted = _submit_stage( session=session, execution=execution, + assessment=assessment, blob=blob, - batch_input=batch_input, bag=bag, stage=nxt, organization_id=organization_id, project_id=project_id, ) + except SubmissionUnavailableError as exc: + # Storage blip, not a bad run: the stage is still PENDING, so retry the tick. + logger.warning( + "[_advance_or_finalize] Submission unreadable, will retry | " + "execution_id=%s | stage=%s | %s", + execution.id, + nxt, + exc, + ) + return {"requeue": True} except Exception as exc: _fail(session, execution, assessment, bag, str(exc)) return {"requeue": False} @@ -808,7 +877,6 @@ def _advance_or_finalize( execution=execution, assessment=assessment, blob=blob, - batch_input=batch_input, bag=bag, stage=nxt, organization_id=organization_id, @@ -825,6 +893,10 @@ def run_batch_stage( Idempotent: keyed off ``stage_status`` in the bag — a redelivery either re-polls the in-flight batch or re-submits a stage that was never dispatched. """ + # Lazy (like callbacks/results below): result_files imports ApiStage from this module. + from app.services.assessment.api.result_files import record_stage_dump + from app.services.assessment.api.submission_store import SubmissionUnavailableError + with Session(engine) as session: execution = session.get(AssessmentRun, execution_id) if execution is None: @@ -853,11 +925,18 @@ def run_batch_stage( ) return {"requeue": False} + # Entry log: distinguishes "the task never ran" from "it ran and did nothing". + logger.info( + "[run_batch_stage] Tick | execution_id=%s | stage=%s | stage_status=%s", + execution_id, + stage, + stage_status, + ) + # Resolving the stored blob can raise (deleted config version -> 404, or an # invalid/old-shape blob). Route these to _fail so the client gets a terminal # callback instead of the run stranding in PROCESSING. try: - batch_input = BatchInput.model_validate(assessment.input) blob = AssessmentConfigBlob.model_validate( _resolve_blob(session, execution, project_id) ) @@ -870,13 +949,23 @@ def run_batch_stage( submitted = _submit_stage( session=session, execution=execution, + assessment=assessment, blob=blob, - batch_input=batch_input, bag=bag, stage=stage, organization_id=organization_id, project_id=project_id, ) + except SubmissionUnavailableError as exc: + # Storage blip, not a bad run: the stage stays PENDING, retry the tick. + logger.warning( + "[run_batch_stage] Submission unreadable, will retry | " + "execution_id=%s | stage=%s | %s", + execution_id, + stage, + exc, + ) + return {"requeue": True} except Exception as exc: # Credential/provider/network errors from _submit_provider_batch, not # just ValueError — all are terminal for this run. @@ -894,7 +983,6 @@ def run_batch_stage( execution=execution, assessment=assessment, blob=blob, - batch_input=batch_input, bag=bag, stage=stage, organization_id=organization_id, @@ -918,7 +1006,9 @@ def run_batch_stage( organization_id=organization_id, project_id=project_id, ) - outcome, results = _poll_outcome(session, provider, batch_job) + outcome, results = _poll_outcome( + session, provider, batch_job, assessment.id + ) except Exception as exc: # Transient (network/provider hiccup) — the batch is still running; retry. logger.warning( @@ -946,7 +1036,22 @@ def run_batch_stage( parsed = parse_batch_results(results or [], bag["provider"]) _record_stage(bag, stage, kind, parsed) bag["stage_status"] = StageStatus.COMPLETED.value + + stage_errors: dict[str, str] = {} + for idx, out in parsed.items(): + error = out.get("error") + if error: + stage_errors[str(idx)] = error + bag.setdefault("stage_errors", {})[stage] = stage_errors + bag.setdefault("stage_output_urls", {})[stage] = batch_job.raw_output_url + # Per stage, not only at terminal time: a run that never terminates still has dumps. + record_stage_dump( + session=session, + assessment=assessment, + stage=stage, + url=batch_job.raw_output_url, + ) api.save_execution_state(session=session, execution=execution, state=bag) return _advance_or_finalize( @@ -954,7 +1059,6 @@ def run_batch_stage( execution=execution, assessment=assessment, blob=blob, - batch_input=batch_input, bag=bag, stage=stage, organization_id=organization_id, diff --git a/backend/app/services/assessment/api/callbacks.py b/backend/app/services/assessment/api/callbacks.py index 169397575..dbea7341c 100644 --- a/backend/app/services/assessment/api/callbacks.py +++ b/backend/app/services/assessment/api/callbacks.py @@ -2,18 +2,21 @@ POSTs the ``AssessmentCallback`` envelope to the client's callback_url on completion via the shared SSRF-guarded ``send_callback`` (HMAC-signed with the project webhook secret) — -the same transport the response path uses. +the same transport the response path uses. One inline attempt, no retry. """ import logging from typing import Any +from sqlmodel import Session + from app.models.assessment import ( Assessment, AssessmentBatchResult, AssessmentCallback, AssessmentStatus, ) +from app.services.assessment.api.result_files import build_callback_metadata from app.utils import get_webhook_secret, send_callback logger = logging.getLogger(__name__) @@ -21,14 +24,17 @@ def deliver( *, + session: Session, assessment: Assessment, result: AssessmentBatchResult, callback_url: str, request_metadata: dict[str, Any] | None, + failure_message: str | None, ) -> bool: """POST the assessment result to ``callback_url`` (HMAC-signed). Returns whether it was sent. - ``request_metadata`` is echoed back unchanged for client-side correlation. + ``request_metadata`` is the client's own echo; the envelope ``metadata`` carries the + presigned result-file urls, and ``failure_message`` becomes the envelope ``error``. """ callback = AssessmentCallback( assessment_id=assessment.id, @@ -39,20 +45,33 @@ def deliver( webhook_secret = get_webhook_secret( assessment.project_id, assessment.organization_id ) + + try: + metadata = build_callback_metadata(session=session, assessment=assessment) + except Exception: + # A metadata bug must never cost the client its result. + logger.error( + "[deliver] Callback metadata failed, delivering without it | assessment_id=%s", + assessment.id, + exc_info=True, + ) + metadata = None + sent = send_callback( callback_url, { "success": assessment.status != AssessmentStatus.FAILED, "data": callback.model_dump(mode="json"), - "error": None, - "metadata": None, + "error": failure_message, + "metadata": metadata, }, webhook_secret=webhook_secret, ) logger.info( - "[deliver] Callback %s | assessment_id=%s | status=%s", + "[deliver] Callback %s | assessment_id=%s | status=%s | result_files=%s", "sent" if sent else "failed", assessment.id, assessment.status, + sorted((metadata or {}).get("result_files", {})), ) return sent diff --git a/backend/app/services/assessment/api/result_files.py b/backend/app/services/assessment/api/result_files.py new file mode 100644 index 000000000..20433ae7e --- /dev/null +++ b/backend/app/services/assessment/api/result_files.py @@ -0,0 +1,322 @@ +"""Durable result dumps for the BATCH API-client path. + +Records every provider dump on ``assessment.result_files`` as ``{kind: {object_store_url}}``, +builds ``errors.jsonl`` at terminal time, and presigns both into the callback envelope. +Nothing here raises into the terminal path: a missing dump degrades to a missing key. +""" + +import json +import logging +from datetime import timedelta +from enum import StrEnum +from typing import Any +from uuid import UUID + +from sqlmodel import Session + +from app.core.cloud import get_cloud_storage +from app.core.config import settings +from app.core.storage_utils import upload_jsonl_to_object_store +from app.core.util import now +from app.crud.assessment import api +from app.crud.job import get_batch_job +from app.models.assessment import ( + Assessment, + AssessmentRun, + BatchRunState, +) +from app.models.batch_job import BatchJob +from app.services.assessment.api.batch import ApiStage, _build_batch_provider + +logger = logging.getLogger(__name__) + +RESULTS_FILE_KIND = "results" +ERRORS_FILE_KIND = "errors" +ERRORS_FILENAME = "errors.jsonl" + +# 86400 is the storage layer's own ceiling, so the presigned urls live exactly one day. +SIGNED_URL_EXPIRY_SECONDS = settings.MAX_SIGNED_URL_EXPIRY_SECONDS + + +class ErrorRecordEnum(StrEnum): + """``type`` tag on each errors.jsonl row, so the file is self-describing.""" + + EXECUTION_ERROR = "execution_error" + ROW_ERROR = "row_error" + PROVIDER_ERROR_FILE = "provider_error_file" + PROVIDER_ERROR_FILE_UNAVAILABLE = "provider_error_file_unavailable" + + +def assessment_subdirectory(assessment_id: UUID) -> str: + """Object-store prefix holding every file one assessment produces.""" + return f"assessment/{assessment_id}" + + +def stage_file_kind(stage: str) -> str: + """Result-file kind for a stage's dump; the assessment dump is the run's ``results``.""" + if stage == ApiStage.ASSESSMENT.value: + return RESULTS_FILE_KIND + return f"{stage}_results" + + +def record_stage_dump( + *, + session: Session, + assessment: Assessment, + stage: str, + url: str | None, +) -> None: + """Record one stage's dump on the parent row as soon as the stage completes. + + No-op on a falsy url: ``process_completed_batch`` swallows a failed upload. + """ + if not url: + logger.warning( + "[record_stage_dump] No dump url to record | assessment_id=%s | stage=%s", + assessment.id, + stage, + ) + return + + api.set_result_files( + session=session, + assessment=assessment, + files={stage_file_kind(stage): {"object_store_url": url}}, + ) + + +def _row_error_rows(stage_errors: dict[str, dict[str, str]]) -> list[dict[str, Any]]: + """Per-row errors captured at parse time, flattened across stages.""" + rows: list[dict[str, Any]] = [] + for stage, errors in stage_errors.items(): + for row_index, error in errors.items(): + rows.append( + { + "type": ErrorRecordEnum.ROW_ERROR.value, + "stage": stage, + "row_index": int(row_index) if row_index.isdigit() else row_index, + "error": error, + } + ) + return rows + + +def _error_file_rows( + *, session: Session, assessment: Assessment, stage: str, batch_job: BatchJob +) -> list[dict[str, Any]]: + """Parsed lines of one stage batch's provider error file (OpenAI only). + + Degrades to a single ``provider_error_file_unavailable`` row: the client still learns + the file existed and could not be read. + """ + file_id = batch_job.provider_error_file_id + if not file_id: + return [] + + try: + provider = _build_batch_provider( + session=session, + provider_name=batch_job.provider, + organization_id=assessment.organization_id, + project_id=assessment.project_id, + ) + content = provider.download_file(file_id) + except Exception as exc: + # Provider SDKs raise heterogeneous types here and the dump is best-effort. + message = ( + f"[KAAPI] Could not download the provider error file " + f"(code: {type(exc).__name__}): {exc}" + ) + logger.error( + "[_error_file_rows] %s | batch_job_id=%s | stage=%s", + message, + batch_job.id, + stage, + exc_info=True, + ) + return [ + { + "type": ErrorRecordEnum.PROVIDER_ERROR_FILE_UNAVAILABLE.value, + "stage": stage, + "provider_error_file_id": file_id, + "error": message, + } + ] + + rows: list[dict[str, Any]] = [] + for line in content.strip().split("\n"): + if not line: + continue + try: + entry = json.loads(line) + except json.JSONDecodeError: + logger.warning( + "[_error_file_rows] Unparseable error-file line, skipping | " + "batch_job_id=%s | stage=%s", + batch_job.id, + stage, + ) + continue + rows.append( + { + "type": ErrorRecordEnum.PROVIDER_ERROR_FILE.value, + "stage": stage, + "provider_error_file_id": file_id, + "entry": entry, + } + ) + return rows + + +def build_and_upload_errors( + *, + session: Session, + execution: AssessmentRun, + assessment: Assessment, + bag: BatchRunState, + failure_message: str | None, +) -> str | None: + """Assemble and upload the run's ``errors.jsonl``. Returns its object-store url. + + Uploaded even when there are no rows, so the "both a results and an errors url" + promise holds on the clean-success path too. + """ + rows: list[dict[str, Any]] = [] + if failure_message: + rows.append( + { + "type": ErrorRecordEnum.EXECUTION_ERROR.value, + "stage": bag.get("stage"), + "error": failure_message, + } + ) + rows.extend(_row_error_rows(bag.get("stage_errors") or {})) + for stage, batch_job_id in (bag.get("stage_batches") or {}).items(): + batch_job = ( + get_batch_job(session=session, batch_job_id=batch_job_id) + if batch_job_id + else None + ) + if batch_job is not None: + rows.extend( + _error_file_rows( + session=session, + assessment=assessment, + stage=stage, + batch_job=batch_job, + ) + ) + + try: + storage = get_cloud_storage(session=session, project_id=assessment.project_id) + except Exception: + logger.error( + "[build_and_upload_errors] Storage unavailable, no errors dump | " + "execution_id=%s | rows=%s", + execution.id, + len(rows), + exc_info=True, + ) + return None + + url = upload_jsonl_to_object_store( + storage=storage, + results=rows, + filename=ERRORS_FILENAME, + subdirectory=assessment_subdirectory(assessment.id), + ) + logger.info( + "[build_and_upload_errors] Errors dump %s | execution_id=%s | rows=%s | url=%s", + "uploaded" if url else "upload failed", + execution.id, + len(rows), + url, + ) + return url + + +def finalize_result_files( + *, + session: Session, + execution: AssessmentRun, + assessment: Assessment, + bag: BatchRunState, + failure_message: str | None = None, +) -> None: + """Persist every stage dump plus the run's errors.jsonl at terminal time. + + Idempotent (a re-merge replaces only its own kind), so a redelivered tick is + harmless. Never raises: durability is best-effort, terminating the run is not. + """ + try: + files: dict[str, dict[str, Any]] = {} + for stage, url in (bag.get("stage_output_urls") or {}).items(): + if not url: + continue + files[stage_file_kind(stage)] = {"object_store_url": url} + + errors_url = build_and_upload_errors( + session=session, + execution=execution, + assessment=assessment, + bag=bag, + failure_message=failure_message, + ) + if errors_url: + files[ERRORS_FILE_KIND] = {"object_store_url": errors_url} + + if files: + api.set_result_files(session=session, assessment=assessment, files=files) + except Exception: + logger.error( + "[finalize_result_files] Could not persist result files | " + "assessment_id=%s | execution_id=%s", + assessment.id, + execution.id, + exc_info=True, + ) + + +def build_callback_metadata( + *, session: Session, assessment: Assessment +) -> dict[str, Any]: + """Presign every recorded result file for the callback envelope's ``metadata``. + + Always returns ``{"result_files": ..., "expires_at": ...}``; a per-key presign + failure drops that entry rather than the whole envelope key. + """ + expires_at = (now() + timedelta(seconds=SIGNED_URL_EXPIRY_SECONDS)).isoformat() + signed: dict[str, dict[str, Any]] = {} + + try: + storage = get_cloud_storage(session=session, project_id=assessment.project_id) + except Exception: + logger.error( + "[build_callback_metadata] Storage unavailable, sending empty result_files | " + "assessment_id=%s", + assessment.id, + exc_info=True, + ) + return {"result_files": signed, "expires_at": expires_at} + + for kind, record in assessment.result_files.items(): + entry: dict[str, Any] = record or {} + object_store_url = entry.get("object_store_url") + if not object_store_url: + continue + try: + signed_url = storage.get_signed_url( + object_store_url, expires_in=SIGNED_URL_EXPIRY_SECONDS + ) + except Exception: + logger.error( + "[build_callback_metadata] Presign failed, dropping kind | " + "assessment_id=%s | kind=%s", + assessment.id, + kind, + exc_info=True, + ) + continue + signed[kind] = {"signed_url": signed_url} + + return {"result_files": signed, "expires_at": expires_at} diff --git a/backend/app/services/assessment/api/results.py b/backend/app/services/assessment/api/results.py index b61973a6d..a34cc399d 100644 --- a/backend/app/services/assessment/api/results.py +++ b/backend/app/services/assessment/api/results.py @@ -20,18 +20,13 @@ AssessmentCounts, AssessmentOutput, AssessmentResult, - BatchInput, BatchRunState, ParsedResult, PreFilter, PreFilterVerdict, Verdict, ) -from app.services.assessment.api.batch import ( - ApiStage, - build_rows, - parse_batch_results, -) +from app.services.assessment.api.batch import ApiStage, parse_batch_results from app.services.assessment.utils.parsing import parse_stored_results logger = logging.getLogger(__name__) @@ -95,12 +90,8 @@ def build_result(*, session: Session, assessment: Assessment) -> AssessmentBatch executions = api.list_executions(session=session, assessment_id=assessment.id) bag = cast(BatchRunState, (executions[0].execution or {}) if executions else {}) - batch_input = ( - BatchInput.model_validate(assessment.input) if assessment.input else None - ) - input_columns = bag.get("input_schema") or {} - rows, _, _ = build_rows(batch_input, input_columns) if batch_input else ([], [], []) - total_items = len(rows) + # Off the execution, not re-derived: the terminal path must not fetch rows to count them. + total_items = executions[0].total_items if executions else 0 gate_passed = bag.get("gate_passed") or [True] * total_items verdicts = bag.get("verdicts") or {} diff --git a/backend/app/services/assessment/api/submission.py b/backend/app/services/assessment/api/submission.py index 6ea711feb..1e3dbe000 100644 --- a/backend/app/services/assessment/api/submission.py +++ b/backend/app/services/assessment/api/submission.py @@ -6,6 +6,7 @@ """ import logging +from uuid import UUID from asgi_correlation_id import correlation_id from fastapi import HTTPException @@ -13,6 +14,8 @@ from sqlmodel import Session from app.crud.assessment import api +from app.crud.assessment.batch import load_submission_file_rows +from app.crud.assessment.submission import get_submission_by_id from app.crud.config import ConfigCrud, ConfigVersionCrud from app.models.assessment import ( AssessmentCreate, @@ -26,6 +29,7 @@ from app.models.config.assessment_blob import AssessmentConfigBlob from app.models.config.config import ConfigTag from app.services.assessment.api import batch as batch_service +from app.services.assessment.api.submission_store import upload_submission_rows from app.utils import validate_callback_url logger = logging.getLogger(__name__) @@ -75,6 +79,41 @@ def _validate_rows_against_schema( ) +def _rows_from_submission( + *, + session: Session, + submission_doc_id: UUID, + organization_id: int, + project_id: int, +) -> BatchInput: + """Read an uploaded submission's rows so the run continues as if they were inline.""" + submission = get_submission_by_id( + session=session, + submission_id=submission_doc_id, + organization_id=organization_id, + project_id=project_id, + ) + try: + rows = load_submission_file_rows(session=session, submission=submission) + except ValueError as exc: + raise HTTPException(status_code=422, detail=str(exc)) from exc + except Exception as exc: + logger.error( + "[_rows_from_submission] Could not read submission | submission_id=%s", + submission_doc_id, + exc_info=True, + ) + raise HTTPException( + status_code=502, detail="Failed to read the submission file from storage." + ) from exc + + if not rows: + raise HTTPException( + status_code=422, detail=f"Submission {submission_doc_id} has no rows." + ) + return BatchInput(data=rows) + + def _resolve_config( *, session: Session, request: AssessmentCreate, project_id: int ) -> tuple[AssessmentConfigBlob, str, str]: @@ -132,7 +171,7 @@ def submit( """Submit a BATCH assessment: persist state, seed the pipeline, dispatch the task.""" from app.celery.tasks.job_execution import run_assessment_api_batch - method = derive_method(request.input, dataset_id=None) + method = derive_method(request.input, submission_id=None) if method != AssessmentMethod.BATCH: # RESPONSE is handled (stubbed) at the route; submit is BATCH-only for now. raise HTTPException( @@ -147,6 +186,15 @@ def submit( raise HTTPException(status_code=422, detail=str(exc)) from exc batch_input: BatchInput = request.input + submission_id = batch_input.submission_doc_id + if submission_id is not None: + batch_input = _rows_from_submission( + session=session, + submission_doc_id=submission_id, + organization_id=organization_id, + project_id=project_id, + ) + blob, provider, model = _resolve_config( session=session, request=request, project_id=project_id ) @@ -169,10 +217,28 @@ def submit( assessment = api.create_assessment( session=session, method=AssessmentMethod.BATCH, - input=batch_input.model_dump(mode="json"), + input=None, + submission_id=submission_id, organization_id=organization_id, project_id=project_id, ) + # Row first: the object key needs its id, and a run without this file is unrunnable. + submission_url = upload_submission_rows( + session=session, + assessment_id=assessment.id, + project_id=project_id, + batch_input=batch_input, + ) + if not submission_url: + api.update_status( + session=session, obj=assessment, status=AssessmentStatus.FAILED + ) + raise HTTPException( + status_code=503, + detail="Failed to store the assessment submission. Please retry.", + ) + api.set_submission_input(session=session, assessment=assessment, url=submission_url) + execution = api.create_execution( session=session, assessment_id=assessment.id, @@ -204,7 +270,7 @@ def submit( trace_id = correlation_id.get() or "" try: - run_assessment_api_batch.delay( + dispatched = run_assessment_api_batch.delay( execution_id=execution.id, organization_id=organization_id, project_id=project_id, @@ -228,9 +294,10 @@ def submit( ) from exc logger.info( "[submit] Dispatched BATCH assessment | assessment_id=%s | execution_id=%s | " - "provider=%s | stages=%s | rows=%s", + "task_id=%s | provider=%s | stages=%s | rows=%s", assessment.id, execution.id, + dispatched.id, provider, [s["stage"] for s in pipeline], total_items, diff --git a/backend/app/services/assessment/api/submission_store.py b/backend/app/services/assessment/api/submission_store.py new file mode 100644 index 000000000..2f3307bf1 --- /dev/null +++ b/backend/app/services/assessment/api/submission_store.py @@ -0,0 +1,70 @@ +"""Object-store round trip for the API-client BATCH submission rows. +""" + +import json +import logging +from uuid import UUID + +from sqlmodel import Session + +from app.core.cloud import get_cloud_storage +from app.core.storage_utils import upload_jsonl_to_object_store +from app.models.assessment import Assessment, BatchInput +from app.services.assessment.api.result_files import assessment_subdirectory + +logger = logging.getLogger(__name__) + +SUBMISSION_FILENAME = "submission.jsonl" + + +class SubmissionUnavailableError(Exception): + """The stored submission rows could not be read; the caller should retry the tick.""" + + +def upload_submission_rows( + *, session: Session, assessment_id: UUID, project_id: int, batch_input: BatchInput +) -> str | None: + """Store the submission rows as JSONL. Returns the object-store url, None on failure.""" + url = upload_jsonl_to_object_store( + storage=get_cloud_storage(session=session, project_id=project_id), + results=batch_input.data, + filename=SUBMISSION_FILENAME, + subdirectory=assessment_subdirectory(assessment_id), + ) + logger.info( + "[upload_submission_rows] Submission %s | assessment_id=%s | rows=%s | url=%s", + "stored" if url else "upload failed", + assessment_id, + len(batch_input.data), + url, + ) + return url + + +def load_submission_rows(*, session: Session, assessment: Assessment) -> BatchInput: + """Stream the stored submission rows back. + + A storage read failure raises ``SubmissionUnavailableError`` so the tick retries: an + S3 blip must not fail a paid-for run. + """ + url = assessment.submission_input + if not url: + raise ValueError( + f"[load_submission_rows] No submission_input on assessment {assessment.id}" + ) + + try: + storage = get_cloud_storage(session=session, project_id=assessment.project_id) + content = storage.stream(url).read().decode("utf-8") + rows = [json.loads(line) for line in content.splitlines() if line.strip()] + except Exception as exc: + raise SubmissionUnavailableError( + f"[load_submission_rows] Could not read {url}: {exc}" + ) from exc + + logger.info( + "[load_submission_rows] Loaded | assessment_id=%s | rows=%s", + assessment.id, + len(rows), + ) + return BatchInput(data=rows) diff --git a/backend/app/services/assessment/mappers.py b/backend/app/services/assessment/mappers.py index 5623222de..46f1049e6 100644 --- a/backend/app/services/assessment/mappers.py +++ b/backend/app/services/assessment/mappers.py @@ -8,6 +8,13 @@ logger = logging.getLogger(__name__) +# Vertex rejects a payload carrying both spellings (the SDK dump emits snake_case). +_SDK_ORDERING_KEY = "property_ordering" +_ORDERING_KEY = "propertyOrdering" + +# Anthropic's output_config.effort ladder; Kaapi's "none"/"minimal" have no equivalent. +ANTHROPIC_EFFORT_LEVELS = ("low", "medium", "high", "xhigh", "max") + def normalize_llm_text(text: str) -> str: if not isinstance(text, str) or not text: @@ -83,8 +90,9 @@ def _convert_json_schema_to_google(schema: dict) -> dict: else normalized_schema ) - if "properties" in google_schema and "propertyOrdering" not in google_schema: - google_schema["propertyOrdering"] = list( + google_schema.pop(_SDK_ORDERING_KEY, None) + if "properties" in google_schema and _ORDERING_KEY not in google_schema: + google_schema[_ORDERING_KEY] = list( normalized_schema.get("required", []) ) or list(google_schema["properties"].keys()) @@ -222,19 +230,35 @@ def map_kaapi_to_anthropic_params(kaapi_params: dict) -> tuple[dict, list[str]]: if max_output_tokens is not None: anthropic_params["max_tokens"] = max_output_tokens + # Structured output and reasoning effort share one container on the Messages API. + output_config: dict[str, object] = {} output_schema = kaapi_params.get("output_schema") if output_schema is not None: - anthropic_params["output_config"] = { - "format": { - "type": "json_schema", - "schema": _ensure_openai_strict_schema(output_schema), - } + output_config["format"] = { + "type": "json_schema", + "schema": _ensure_openai_strict_schema(output_schema), } - if kaapi_params.get("effort") or kaapi_params.get("reasoning"): + effort = kaapi_params.get("effort") or kaapi_params.get("reasoning") + if effort in ANTHROPIC_EFFORT_LEVELS: + output_config["effort"] = effort + elif effort is not None: + warnings.append( + f"Parameter 'effort' value '{effort}' is not an Anthropic effort level " + f"({', '.join(ANTHROPIC_EFFORT_LEVELS)}) and was ignored." + ) + + if output_config: + anthropic_params["output_config"] = output_config + + thinking = kaapi_params.get("thinking") + if thinking is not None: + anthropic_params["thinking"] = thinking + + if kaapi_params.get("thinking_level") is not None: warnings.append( - "Parameters 'effort'/'reasoning' are not mapped for Anthropic " - "batch assessment and were ignored." + "Parameter 'thinking_level' is Google-only; Anthropic reads the 'thinking' " + "container instead, so it was ignored." ) if kaapi_params.get("knowledge_base_ids"): diff --git a/backend/app/services/assessment/service.py b/backend/app/services/assessment/service.py index 888a949bb..6a7724d82 100644 --- a/backend/app/services/assessment/service.py +++ b/backend/app/services/assessment/service.py @@ -2,6 +2,7 @@ import logging from typing import Any +from uuid import UUID from asgi_correlation_id import correlation_id from fastapi import HTTPException @@ -10,7 +11,7 @@ from app.crud.assessment import ( create_assessment, create_assessment_run, - get_assessment_dataset_by_id, + get_submission_by_id, get_assessment_runs_for_assessment, recompute_assessment_status, ) @@ -51,7 +52,7 @@ def _build_retry_request( *, experiment_name: str, - dataset_id: int, + submission_id: UUID, input_binding: dict[str, Any] | None, runs: list[AssessmentRun], ) -> AssessmentRunCreate: @@ -87,7 +88,7 @@ def _build_retry_request( return AssessmentRunCreate( experiment_name=experiment_name, - dataset_id=dataset_id, + submission_id=submission_id, input_binding=binding, configs=configs, post_processing_config=input_binding.get("post_processing_config"), @@ -108,16 +109,16 @@ def start_assessment( from app.celery.tasks.job_execution import run_assessment_pipeline logger.info( - "[start_assessment] Starting | experiment=%s | dataset_id=%s | configs=%s | org_id=%s", + "[start_assessment] Starting | experiment=%s | submission_id=%s | configs=%s | org_id=%s", request.experiment_name, - request.dataset_id, + request.submission_id, len(request.configs), organization_id, ) - dataset = get_assessment_dataset_by_id( + submission = get_submission_by_id( session=session, - dataset_id=request.dataset_id, + submission_id=request.submission_id, organization_id=organization_id, project_id=project_id, ) @@ -177,7 +178,7 @@ def start_assessment( assessment = create_assessment( session=session, experiment_name=request.experiment_name, - dataset_id=request.dataset_id, + submission_id=request.submission_id, organization_id=organization_id, project_id=project_id, input_binding=assessment_input, @@ -220,8 +221,8 @@ def start_assessment( return AssessmentRunResponse( assessment_id=assessment.id, experiment_name=request.experiment_name, - dataset_id=request.dataset_id, - dataset_name=dataset.name, + submission_id=request.submission_id, + submission_name=submission.name, num_configs=len(runs), runs=[ AssessmentRunSummary( @@ -248,7 +249,7 @@ def retry_assessment( ) request = _build_retry_request( experiment_name=assessment.experiment_name, - dataset_id=assessment.dataset_id, + submission_id=assessment.submission_id, input_binding=assessment.input, runs=runs, ) @@ -277,7 +278,7 @@ def retry_assessment_run( ) request = _build_retry_request( experiment_name=parent.experiment_name, - dataset_id=parent.dataset_id, + submission_id=parent.submission_id, input_binding=parent.input, runs=[run], ) @@ -319,9 +320,9 @@ def resume_assessment_run( status_code=404, detail=f"Parent assessment {run.assessment_id} not found", ) - dataset = get_assessment_dataset_by_id( + submission = get_submission_by_id( session=session, - dataset_id=parent.dataset_id, + submission_id=parent.submission_id, organization_id=organization_id, project_id=project_id, ) @@ -349,8 +350,8 @@ def resume_assessment_run( return AssessmentRunResponse( assessment_id=parent.id, experiment_name=parent.experiment_name, - dataset_id=parent.dataset_id, - dataset_name=dataset.name if dataset else None, + submission_id=parent.submission_id, + submission_name=submission.name if submission else None, num_configs=1, runs=[ AssessmentRunSummary( diff --git a/backend/app/services/assessment/stages.py b/backend/app/services/assessment/stages.py index 0676ce05e..5b7ac9fe4 100644 --- a/backend/app/services/assessment/stages.py +++ b/backend/app/services/assessment/stages.py @@ -63,13 +63,24 @@ def build_pipeline(assessment_input: dict[str, Any]) -> dict[str, Any]: return {"stages": stages} -def ordered_stages(pipeline: dict[str, Any] | None) -> list[str]: - """The stage names in execution order.""" - return [s["stage"] for s in (pipeline or {}).get("stages", [])] +def ordered_stages(pipeline: dict[str, Any] | list[dict[str, str]] | None) -> list[str]: + """The stage names in execution order, for the legacy RUN pipeline shape only. + + The BATCH API path writes a bare list into the same JSONB column, so name the bad + shape here instead of failing with an AttributeError inside the caller. + """ + if pipeline is None: + return [] + if not isinstance(pipeline, dict): + raise ValueError( + f"[ordered_stages] Expected the RUN pipeline mapping " + f"{{'stages': [...]}}, got {type(pipeline).__name__}" + ) + return [s["stage"] for s in pipeline.get("stages", [])] def next_stage( - pipeline: dict[str, Any] | None, current: str | None = None + pipeline: dict[str, Any] | list[dict[str, str]] | None, current: str | None = None ) -> str | None: """First stage when ``current`` is None, else the stage after it (None if last).""" stages = ordered_stages(pipeline) diff --git a/backend/app/services/assessment/dataset.py b/backend/app/services/assessment/submission.py similarity index 70% rename from backend/app/services/assessment/dataset.py rename to backend/app/services/assessment/submission.py index 22ebaae70..9658034ca 100644 --- a/backend/app/services/assessment/dataset.py +++ b/backend/app/services/assessment/submission.py @@ -1,20 +1,21 @@ -"""Dataset management service for assessments (CSV + XLSX). +"""Uploaded submission files for assessments (CSV + XLSX). -Upload stores files directly to object store as-is (no column validation, -no format conversion). Row count is computed for metadata. +Stored as-is: no column validation, no format conversion. The row count is computed at +upload so nothing has to re-read the file to learn how many rows it holds. """ import csv import io import logging +from pathlib import Path from fastapi import HTTPException from sqlmodel import Session from app.core.cloud import get_cloud_storage -from app.core.storage_utils import generate_timestamped_filename, upload_to_object_store -from app.crud.assessment.dataset import create_assessment_dataset -from app.models.evaluation import EvaluationDataset +from app.core.storage_utils import upload_to_object_store +from app.crud.assessment.submission import create_submission, get_submission_by_name +from app.models.assessment import AssessmentSubmission from app.services.evaluations.validators import sanitize_dataset_name logger = logging.getLogger(__name__) @@ -27,22 +28,28 @@ class InvalidFileException(Exception): pass +SUBMISSIONS_SUBDIRECTORY = "assessment/submissions" + _MIME_TYPES = { ".csv": "text/csv", ".xlsx": "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", } +def file_extension_of(object_store_url: str) -> str: + """Format of a stored submission, read off its key (no column holds it).""" + return Path(object_store_url).suffix.lower() + + def _upload_file_to_object_store( session: Session, project_id: int, file_content: bytes, file_ext: str, - dataset_name: str, + submission_name: str, ) -> str | None: """Upload the raw file to object store, preserving original format.""" - extension = file_ext.lstrip(".") - filename = generate_timestamped_filename(dataset_name, extension=extension) + filename = f"{submission_name}.{file_ext.lstrip('.')}" content_type = _MIME_TYPES.get(file_ext, "application/octet-stream") try: @@ -51,7 +58,7 @@ def _upload_file_to_object_store( storage=storage, content=file_content, filename=filename, - subdirectory="datasets", + subdirectory=SUBMISSIONS_SUBDIRECTORY, content_type=content_type, ) except Exception as e: @@ -182,20 +189,19 @@ def _preview_excel(content: bytes, limit: int) -> tuple[list[str], list[list[str wb.close() -def preview_dataset( +def preview_submission( session: Session, - dataset: EvaluationDataset, + submission: AssessmentSubmission, project_id: int, limit: int, ) -> tuple[list[str], list[list[str]]]: - """Return the first `limit` data rows (plus header) of a dataset file.""" - if not dataset.object_store_url: + """Return the first `limit` data rows (plus header) of a submission file.""" + if not submission.object_store_url: raise HTTPException( - status_code=404, detail="Dataset has no underlying file to preview." + status_code=404, detail="Submission has no underlying file to preview." ) - raw_ext = (dataset.dataset_metadata or {}).get("file_extension") - file_ext = raw_ext.strip().lower() if isinstance(raw_ext, str) else None + file_ext = file_extension_of(submission.object_store_url) if file_ext == ".xls": raise HTTPException( status_code=422, @@ -209,14 +215,14 @@ def preview_dataset( storage = get_cloud_storage(session=session, project_id=project_id) try: - content = storage.get(dataset.object_store_url) + content = storage.get(submission.object_store_url) except Exception as e: logger.warning( - f"[preview_dataset] Failed to fetch file | dataset_id={dataset.id} | {e}", + f"[preview_submission] Failed to fetch file | submission_id={submission.id} | {e}", exc_info=True, ) raise HTTPException( - status_code=502, detail="Failed to fetch dataset file from storage." + status_code=502, detail="Failed to fetch the submission file from storage." ) from e try: @@ -227,33 +233,50 @@ def preview_dataset( raise HTTPException(status_code=422, detail="Invalid XLSX file content.") from e except Exception as e: logger.warning( - f"[preview_dataset] Failed to parse file | dataset_id={dataset.id} | {e}", + f"[preview_submission] Failed to parse file | submission_id={submission.id} | {e}", exc_info=True, ) raise HTTPException( - status_code=422, detail="Unable to parse dataset file for preview." + status_code=422, detail="Unable to parse the submission file for preview." ) from e -def upload_dataset( +def upload_submission( session: Session, file_content: bytes, file_ext: str, - dataset_name: str, + submission_name: str, description: str | None, organization_id: int, project_id: int, -) -> EvaluationDataset: - """Upload a dataset file directly to object store and record metadata.""" - original_name = dataset_name +) -> AssessmentSubmission: + """Store an uploaded submission file and record it.""" + original_name = submission_name try: - dataset_name = sanitize_dataset_name(dataset_name) + submission_name = sanitize_dataset_name(submission_name) except ValueError as e: - raise HTTPException(status_code=422, detail=f"Invalid dataset name: {str(e)}") + raise HTTPException( + status_code=422, detail=f"Invalid submission name: {str(e)}" + ) - if original_name != dataset_name: + if original_name != submission_name: logger.info( - f"[upload_dataset] Dataset name sanitized | '{original_name}' -> '{dataset_name}'" + f"[upload_submission] Name sanitized | '{original_name}' -> '{submission_name}'" + ) + + # Before the upload: the key is the name, so a late reject would have overwritten it. + if get_submission_by_name( + session=session, + name=submission_name, + organization_id=organization_id, + project_id=project_id, + ): + raise HTTPException( + status_code=409, + detail=( + f"Submission with name '{submission_name}' already exists in this " + "organization and project." + ), ) try: @@ -268,11 +291,11 @@ def upload_dataset( except Exception as e: raise HTTPException( status_code=422, - detail="Unable to parse dataset file. Please upload a valid CSV or XLSX file.", + detail="Unable to parse the file. Please upload a valid CSV or XLSX file.", ) from e logger.info( - f"[upload_dataset] Uploading dataset | dataset={dataset_name} | " + f"[upload_submission] Uploading | name={submission_name} | " f"file_type={file_ext} | rows={row_count} | " f"org_id={organization_id} | project_id={project_id}" ) @@ -282,38 +305,31 @@ def upload_dataset( project_id=project_id, file_content=file_content, file_ext=file_ext, - dataset_name=dataset_name, + submission_name=submission_name, ) if not object_store_url: logger.error( - f"[upload_dataset] Object store upload failed | dataset={dataset_name} | " + f"[upload_submission] Object store upload failed | name={submission_name} | " f"org_id={organization_id} | project_id={project_id}" ) raise HTTPException( status_code=500, - detail="Failed to upload dataset file. Please try again.", + detail="Failed to upload the submission file. Please try again.", ) - metadata = { - "file_extension": file_ext, - "file_size_bytes": len(file_content), - "total_items_count": row_count, - } - - dataset = create_assessment_dataset( + submission = create_submission( session=session, - name=dataset_name, + name=submission_name, description=description, - dataset_metadata=metadata, object_store_url=object_store_url, - langfuse_dataset_id=None, + total_items=row_count, organization_id=organization_id, project_id=project_id, ) logger.info( - f"[upload_dataset] Created dataset record | " - f"id={dataset.id} | name={dataset_name} | rows={row_count}" + f"[upload_submission] Created record | " + f"id={submission.id} | name={submission_name} | rows={row_count}" ) - return dataset + return submission diff --git a/backend/app/services/assessment/tasks.py b/backend/app/services/assessment/tasks.py index 33e066385..732046a65 100644 --- a/backend/app/services/assessment/tasks.py +++ b/backend/app/services/assessment/tasks.py @@ -9,11 +9,11 @@ from app.celery.tasks.job_execution import run_assessment_pipeline from app.core.db import engine from app.crud.assessment import ( - get_assessment_dataset_by_id, + get_submission_by_id, recompute_assessment_status, update_assessment_run_status, ) -from app.crud.assessment.batch import _load_dataset_rows, submit_assessment_batch +from app.crud.assessment.batch import load_submission_file_rows, submit_assessment_batch from app.crud.assessment.core import _read_exec, _write_exec from app.crud.assessment.processing import parse_assessment_output from app.crud.evaluations.core import resolve_evaluation_config @@ -108,13 +108,13 @@ def _dispatch(run_id: int, organization_id: int, project_id: int) -> None: def _resolve_run_context( session: Session, run: AssessmentRun, organization_id: int, project_id: int ): - """Load the assessment, dataset, and resolved config; ``error`` set on failure.""" + """Load the assessment, submission, and resolved config; ``error`` set on failure.""" assessment = session.get(Assessment, run.assessment_id) if assessment is None: return None, None, None, "Parent assessment not found." - dataset = get_assessment_dataset_by_id( + submission = get_submission_by_id( session=session, - dataset_id=assessment.dataset_id, + submission_id=assessment.submission_id, organization_id=organization_id, project_id=project_id, ) @@ -126,8 +126,8 @@ def _resolve_run_context( tag=ConfigTag.ASSESSMENT, ) if error or config_blob is None: - return assessment, dataset, None, f"Config resolution failed: {error}" - return assessment, dataset, config_blob, None + return assessment, submission, None, f"Config resolution failed: {error}" + return assessment, submission, config_blob, None def _accepted_indices( @@ -201,7 +201,7 @@ def _orchestrate(run_id: int, organization_id: int, project_id: int) -> None: def _submit_stage( session: Session, run: AssessmentRun, organization_id: int, project_id: int ) -> None: - assessment, dataset, config_blob, error = _resolve_run_context( + assessment, submission, config_blob, error = _resolve_run_context( session, run, organization_id, project_id ) if error: @@ -212,7 +212,7 @@ def _submit_stage( recompute_assessment_status(session=session, assessment_id=run.assessment_id) return - all_rows = _load_dataset_rows(session, dataset) + all_rows = load_submission_file_rows(session=session, submission=submission) if not all_rows: _write_exec(run, stage_status=StageStatus.FAILED) update_assessment_run_status( @@ -269,7 +269,7 @@ def _submit_stage( session=session, run=run, assessment=assessment, - dataset=dataset, + submission=submission, config_blob=config_blob, assessment_input=assessment_input, organization_id=organization_id, diff --git a/backend/app/services/assessment/utils/export.py b/backend/app/services/assessment/utils/export.py index 64110380c..7302e4785 100644 --- a/backend/app/services/assessment/utils/export.py +++ b/backend/app/services/assessment/utils/export.py @@ -22,10 +22,10 @@ AssessmentExportRow, AssessmentRun, AssessmentStatus, + AssessmentSubmission, Stage, ) from app.models.batch_job import BatchJob -from app.models.evaluation import EvaluationDataset from app.services.assessment.prefilter.duplicate_detection import ( parse_duplicate_detection_results, ) @@ -43,15 +43,15 @@ logger = logging.getLogger(__name__) -def _load_dataset_rows( +def _load_submission_rows( session: Session, - dataset: EvaluationDataset, + submission: AssessmentSubmission, ) -> list[dict[str, str]]: # Imported lazily: app.crud.assessment.batch pulls this module via # app.services.assessment.utils, so a top-level import would be circular. - from app.crud.assessment.batch import _load_dataset_rows as load_dataset_rows + from app.crud.assessment.batch import load_submission_file_rows - return load_dataset_rows(session, dataset) + return load_submission_file_rows(session=session, submission=submission) def _stage_batch_job( @@ -113,7 +113,7 @@ def _expand_input_columns( ) -> tuple[list[dict[str, Any]], list[str]]: """Expand ``input_data`` dict into separate input columns. - Uses the original column names from the dataset (no prefix). + Uses the original column names from the submission (no prefix). Returns: (expanded_rows with input_data replaced by individual columns, @@ -144,7 +144,7 @@ def _expand_input_columns( collisions = {key: value for key, value in key_map.items() if key != value} if collisions: logger.warning( - "[_expand_input_columns] Input dataset columns conflict with reserved " + "[_expand_input_columns] Input submission columns conflict with reserved " "export fields and were namespaced: %s", collisions, ) @@ -450,27 +450,27 @@ def _load_parsed_results_for_run( return None -def _load_dataset_rows_for_run( +def _load_submission_rows_for_run( session: Session, run: AssessmentRun, assessment: Assessment, ) -> list[dict[str, str]]: - """Load original dataset rows for input-output correlation. + """Load the original submission rows for input-output correlation. - Returns an empty list if the dataset is not available. + Returns an empty list if the submission is not available. """ try: - dataset = session.get(EvaluationDataset, assessment.dataset_id) - if not dataset or not dataset.object_store_url: + submission = session.get(AssessmentSubmission, assessment.submission_id) + if not submission or not submission.object_store_url: logger.warning( - "[_load_dataset_rows_for_run] Dataset not available for run id=%s", + "[_load_submission_rows_for_run] Submission not available for run id=%s", run.id, ) return [] - return _load_dataset_rows(session, dataset) + return _load_submission_rows(session, submission) except Exception as exc: logger.warning( - "[_load_dataset_rows_for_run] Failed to load dataset for run id=%s: %s", + "[_load_submission_rows_for_run] Failed to load submission for run id=%s: %s", run.id, exc, ) @@ -594,13 +594,13 @@ def load_export_rows_for_run( ) return [] - dataset_rows = _load_dataset_rows_for_run(session, run, assessment) + submission_rows = _load_submission_rows_for_run(session, run, assessment) prefilter_by_row_id = _load_prefilter_results(session, run, assessment) l2_by_row_id = _load_l2_results_for_run(session, run, assessment) has_prefilter = bool(prefilter_by_row_id) - if dataset_rows: + if submission_rows: rows = [ _build_export_row( run=run, @@ -611,7 +611,7 @@ def load_export_rows_for_run( l2_item=l2_by_row_id.get(f"row_{row_idx}"), has_prefilter=has_prefilter, ) - for row_idx, input_data in enumerate(dataset_rows) + for row_idx, input_data in enumerate(submission_rows) ] return rows diff --git a/backend/app/services/llm/mappers.py b/backend/app/services/llm/mappers.py index f2e3d7d05..295c06a07 100644 --- a/backend/app/services/llm/mappers.py +++ b/backend/app/services/llm/mappers.py @@ -36,6 +36,10 @@ "tts": DEFAULT_ELEVENLABS_TTS_MODEL, } +# Vertex rejects a payload carrying both spellings (the SDK dump emits snake_case). +_SDK_ORDERING_KEY = "property_ordering" +_ORDERING_KEY = "propertyOrdering" + logger = logging.getLogger(__name__) @@ -127,8 +131,9 @@ def _convert_json_schema_to_google(schema: dict[str, Any]) -> dict[str, Any]: else normalized_schema ) - if "properties" in google_schema and "propertyOrdering" not in google_schema: - google_schema["propertyOrdering"] = list( + google_schema.pop(_SDK_ORDERING_KEY, None) + if "properties" in google_schema and _ORDERING_KEY not in google_schema: + google_schema[_ORDERING_KEY] = list( normalized_schema.get("required", []) ) or list(google_schema["properties"].keys()) diff --git a/backend/app/tests/assessment/test_api_crud.py b/backend/app/tests/assessment/test_api_crud.py index e41e76eee..1ad7502e9 100644 --- a/backend/app/tests/assessment/test_api_crud.py +++ b/backend/app/tests/assessment/test_api_crud.py @@ -162,6 +162,63 @@ def test_list_executions_orders_by_id(self, db) -> None: assert [e.id for e in listed] == [first.id, second.id] +class TestSetResultFiles: + def _assessment(self, db, auth): + return api.create_assessment( + session=db, + method=AssessmentMethod.BATCH, + input={"data": [{"a": "1"}]}, + organization_id=auth.organization_id, + project_id=auth.project_id, + ) + + def test_second_kind_does_not_evict_the_first(self, db) -> None: + auth = get_user_test_auth_context(db) + assessment = self._assessment(db, auth) + + api.set_result_files( + session=db, + assessment=assessment, + files={"results": {"url": "s3://b/out.jsonl", "count": 998}}, + ) + api.set_result_files( + session=db, + assessment=assessment, + files={"errors": {"url": "s3://b/errors.jsonl", "count": 389}}, + ) + + db.refresh(assessment) + assert assessment.result_files == { + "results": {"url": "s3://b/out.jsonl", "count": 998}, + "errors": {"url": "s3://b/errors.jsonl", "count": 389}, + } + + def test_rerecording_a_kind_replaces_only_that_kind(self, db) -> None: + auth = get_user_test_auth_context(db) + assessment = self._assessment(db, auth) + + api.set_result_files( + session=db, + assessment=assessment, + files={ + "results": {"url": "s3://b/stale.jsonl", "count": 1}, + "errors": {"url": "s3://b/errors.jsonl", "count": 389}, + }, + ) + api.set_result_files( + session=db, + assessment=assessment, + files={"results": {"url": "s3://b/fresh.jsonl", "count": 998}}, + ) + + db.refresh(assessment) + assert assessment.result_files["results"] == { + "url": "s3://b/fresh.jsonl", + "count": 998, + } + assert assessment.result_files["errors"]["count"] == 389 + + class TestDeriveMethod: def test_response_input(self) -> None: assert derive_method(ResponseInput(), None) == AssessmentMethod.RESPONSE @@ -257,6 +314,16 @@ def test_user_set_temperature_kept(self) -> None: blob = AssessmentConfigBlob.model_validate(_assessment_params(temperature=0.4)) assert blob.assessment.params["temperature"] == 0.4 + def test_thinking_params_reach_the_stored_params(self) -> None: + # validate_params replaces params with the model dump, so a param the model + # does not declare is silently dropped before any mapper can see it. + thinking = {"type": "enabled", "budget_tokens": 4096} + blob = AssessmentConfigBlob.model_validate( + _assessment_params(thinking=thinking, thinking_level="high") + ) + assert blob.assessment.params["thinking"] == thinking + assert blob.assessment.params["thinking_level"] == "high" + class TestInputSchemaValidators: def test_missing_input_schema_rejected(self) -> None: diff --git a/backend/app/tests/assessment/test_cron.py b/backend/app/tests/assessment/test_cron.py index e2fb211b4..38cdedc5c 100644 --- a/backend/app/tests/assessment/test_cron.py +++ b/backend/app/tests/assessment/test_cron.py @@ -5,11 +5,32 @@ import pytest +from app.crud.assessment import api as assessment_api +from app.crud.assessment import core as assessment_core from app.crud.assessment.cron import ( _log_config_progress, poll_all_pending_assessment_evaluations, ) -from app.models.assessment import StageStatus +from app.models.assessment import AssessmentMethod, AssessmentStatus, StageStatus +from app.models.config.assessment_blob import AssessmentConfigBlob +from app.models.config.config import ConfigTag +from app.tests.utils.auth import get_user_test_auth_context +from app.tests.utils.test_data import ( + create_test_config, + create_test_evaluation_dataset, +) +from app.tests.utils.utils import random_lower_string + +_ASSESSMENT_BLOB = AssessmentConfigBlob.model_validate( + { + "input_schema": {"a": {"type": "text"}}, + "assessment": { + "provider": "openai", + "type": "text", + "params": {"model": "gpt-4o", "submission": "assess {a}"}, + }, + } +) @pytest.fixture(autouse=True) @@ -166,6 +187,35 @@ async def test_transient_poll_exception_does_not_fail_run(self) -> None: assert result["failed"] == 0 assert result["still_processing"] == 1 + @pytest.mark.asyncio + async def test_attribute_error_marks_run_failed(self) -> None: + """The I-2 shape bug surfaced as an AttributeError and looped forever on retry. + + Session is a mock because the failure branch calls ``session.rollback()``, which + unwinds the ``db`` fixture's outer transaction along with the seeded rows. + """ + session = MagicMock() + assessment = _make_assessment(id=1, status="processing") + run = _make_run(id=11, execution={"stage_status": StageStatus.PROCESSING}) + session.exec.return_value.all.return_value = [assessment] + + with patch( + "app.crud.assessment.cron.get_assessment_runs_for_assessment", + return_value=[run], + ), patch( + "app.crud.assessment.cron.process_run_batches", + new=AsyncMock( + side_effect=AttributeError("'list' object has no attribute 'get'") + ), + ), patch( + "app.crud.assessment.cron.update_assessment_run_status" + ) as mark_failed: + result = await poll_all_pending_assessment_evaluations(session=session) + + assert result["failed"] == 1 + assert result["still_processing"] == 0 + assert mark_failed.call_args.kwargs["status"] == "FAILED" + @pytest.mark.asyncio async def test_deterministic_error_marks_run_failed(self) -> None: """A deterministic ValueError fails the run instead of retrying forever.""" @@ -188,3 +238,84 @@ async def test_deterministic_error_marks_run_failed(self) -> None: assert result["failed"] == 1 assert result["still_processing"] == 0 assert mark_failed.call_args.kwargs["status"] == "FAILED" + + +class TestPollerAgainstRealRows: + """The poller's method boundary and its failure classification, on real rows.""" + + def _run_assessment(self, db, auth): + dataset = create_test_evaluation_dataset( + db, organization_id=auth.organization_id, project_id=auth.project_id + ) + return assessment_core.create_assessment( + session=db, + experiment_name="exp", + dataset_id=dataset.id, + organization_id=auth.organization_id, + project_id=auth.project_id, + ) + + def _batch_assessment(self, db, auth): + return assessment_api.create_assessment( + session=db, + method=AssessmentMethod.BATCH, + input={"data": [{"a": "1"}]}, + organization_id=auth.organization_id, + project_id=auth.project_id, + ) + + @pytest.mark.asyncio + async def test_batch_assessments_are_not_polled_but_run_ones_are(self, db) -> None: + auth = get_user_test_auth_context(db) + run_assessment = self._run_assessment(db, auth) + batch_assessment = self._batch_assessment(db, auth) + + polled: list = [] + + def record(*, session, assessment_id): + polled.append(assessment_id) + return [] + + with patch( + "app.crud.assessment.cron.get_assessment_runs_for_assessment", + side_effect=record, + ): + await poll_all_pending_assessment_evaluations(session=db) + + assert run_assessment.id in polled + assert batch_assessment.id not in polled + + @pytest.mark.asyncio + async def test_a_run_assessments_active_run_is_polled(self, db) -> None: + auth = get_user_test_auth_context(db) + assessment = self._run_assessment(db, auth) + config = create_test_config( + db, + project_id=auth.project_id, + name=f"assess-{random_lower_string()}", + config_blob=_ASSESSMENT_BLOB, + tag=ConfigTag.ASSESSMENT, + ) + run = assessment_core.create_assessment_run( + session=db, + assessment_id=assessment.id, + config_id=config.id, + config_version=1, + ) + assessment_core.update_assessment_run_status( + session=db, run=run, status=AssessmentStatus.PROCESSING + ) + assessment_core._write_exec(run, stage_status=StageStatus.PROCESSING) + db.add(run) + db.commit() + + polled_run_ids: list[int] = [] + + async def record(*, run, session): + polled_run_ids.append(run.id) + return {"action": "still_processing"} + + with patch("app.crud.assessment.cron.process_run_batches", new=record): + await poll_all_pending_assessment_evaluations(session=db) + + assert run.id in polled_run_ids diff --git a/backend/app/tests/assessment/test_mappers.py b/backend/app/tests/assessment/test_mappers.py index 4ebb636a2..b4a445cfe 100644 --- a/backend/app/tests/assessment/test_mappers.py +++ b/backend/app/tests/assessment/test_mappers.py @@ -259,6 +259,29 @@ def test_property_ordering_falls_back_to_keys(self) -> None: assert "propertyOrdering" in result +ENUM_OUTPUT_SCHEMA = { + "type": "object", + "properties": { + "band": {"type": "string", "enum": ["low", "high"]}, + "score": {"type": "integer"}, + }, + "required": ["band", "score"], +} + + +class TestGoogleSchemaOrderingKey: + def test_enum_schema_carries_only_the_camel_case_ordering_key(self) -> None: + # Real SDK transformer, not the stub above: it only emits the snake_case + # property_ordering for some schemas (an enum triggers it), and Vertex + # rejects a payload carrying both spellings. + result, _ = map_kaapi_to_google_params( + {"model": "gemini-2.5-pro", "output_schema": ENUM_OUTPUT_SCHEMA} + ) + google_schema = result["output_schema"] + assert "property_ordering" not in google_schema + assert google_schema["propertyOrdering"] == ["band", "score"] + + class TestOpenAIResponseFormat: def _call(self, params: dict): with patch( @@ -318,7 +341,7 @@ def test_output_schema_maps_to_output_config(self) -> None: assert fmt["type"] == "json_schema" assert fmt["schema"]["additionalProperties"] is False - def test_unsupported_params_warned(self) -> None: + def test_knowledge_base_ids_warned(self) -> None: result, warnings = self._call( { "model": "claude-sonnet-4-6", @@ -326,6 +349,49 @@ def test_unsupported_params_warned(self) -> None: "knowledge_base_ids": ["kb1"], } ) - assert "effort" not in result - assert any("effort" in w for w in warnings) + assert "knowledge_base_ids" not in result assert any("knowledge_base_ids" in w for w in warnings) + + def test_effort_lands_in_output_config_without_a_schema(self) -> None: + result, warnings = self._call({"model": "claude-sonnet-4-6", "effort": "high"}) + assert result["output_config"] == {"effort": "high"} + assert warnings == [] + + def test_effort_shares_the_output_config_with_the_schema(self) -> None: + schema = {"type": "object", "properties": {"score": {"type": "integer"}}} + result, _ = self._call( + { + "model": "claude-sonnet-4-6", + "effort": "high", + "output_schema": schema, + } + ) + assert result["output_config"]["effort"] == "high" + assert result["output_config"]["format"]["type"] == "json_schema" + + def test_reasoning_is_read_as_effort(self) -> None: + result, _ = self._call({"model": "claude-sonnet-4-6", "reasoning": "medium"}) + assert result["output_config"]["effort"] == "medium" + + def test_minimal_effort_is_warned_and_omitted(self) -> None: + # "minimal" is a Kaapi/OpenAI rung with no Anthropic equivalent. + result, warnings = self._call( + {"model": "claude-sonnet-4-6", "effort": "minimal"} + ) + assert "output_config" not in result + assert any("minimal" in w for w in warnings) + + def test_thinking_container_passes_through(self) -> None: + thinking = {"type": "enabled", "budget_tokens": 4096} + result, warnings = self._call( + {"model": "claude-sonnet-4-6", "thinking": thinking} + ) + assert result["thinking"] == thinking + assert warnings == [] + + def test_thinking_level_is_warned_and_dropped(self) -> None: + result, warnings = self._call( + {"model": "claude-sonnet-4-6", "thinking_level": "high"} + ) + assert "thinking_level" not in result + assert any("thinking_level" in w for w in warnings) diff --git a/backend/app/tests/assessment/test_pipeline.py b/backend/app/tests/assessment/test_pipeline.py index fbdeb7950..a41b5095d 100644 --- a/backend/app/tests/assessment/test_pipeline.py +++ b/backend/app/tests/assessment/test_pipeline.py @@ -76,6 +76,16 @@ def test_next_stage(self) -> None: ) assert next_stage(pipeline, Stage.L2_ASSESSMENT) is None + def test_no_pipeline_is_no_stages(self) -> None: + assert ordered_stages(None) == [] + + def test_batch_api_list_pipeline_names_the_bad_shape(self) -> None: + # The BATCH API path writes a bare list into the same JSONB column; the legacy + # poller used to hit `AttributeError: 'list' object has no attribute 'get'` here. + batch_pipeline = [{"stage": "assessment", "kind": "ASSESSMENT"}] + with pytest.raises(ValueError, match="got list"): + ordered_stages(batch_pipeline) + class TestAdvanceOrFinalize: def test_advances_to_next_pending_stage(self) -> None: diff --git a/backend/app/tests/assessment/test_result_files.py b/backend/app/tests/assessment/test_result_files.py new file mode 100644 index 000000000..42676ca09 --- /dev/null +++ b/backend/app/tests/assessment/test_result_files.py @@ -0,0 +1,564 @@ +"""Tests for the durable result-file dumps (app/services/assessment/api/result_files.py). + +Real assessment/execution/batch_job rows on the transactional session; object storage +and the provider client are the only seams stubbed. +""" + +import json +from datetime import datetime, timedelta +from unittest.mock import MagicMock, patch + +from app.core.util import now +from app.crud.assessment import api +from app.models.assessment import AssessmentMethod, BatchRunState +from app.models.batch_job import BatchJob, BatchJobType +from app.models.config.assessment_blob import AssessmentConfigBlob +from app.models.config.config import ConfigTag +from app.services.assessment.api.batch import ApiStage +from app.services.assessment.api.result_files import ( + build_and_upload_errors, + build_callback_metadata, + finalize_result_files, + record_stage_dump, + stage_file_kind, +) +from app.tests.utils.auth import get_user_test_auth_context +from app.tests.utils.test_data import create_test_config +from app.tests.utils.utils import random_lower_string + +ONE_DAY_SECONDS = 86400 + +_BLOB = AssessmentConfigBlob.model_validate( + { + "input_schema": {"a": {"type": "text"}}, + "assessment": { + "provider": "openai", + "type": "text", + "params": {"model": "gpt-4o", "submission": "assess {a}"}, + }, + } +) + + +def _bag(**overrides) -> BatchRunState: + bag: dict = { + "pipeline": [{"stage": ApiStage.ASSESSMENT.value, "kind": "ASSESSMENT"}], + "stage": ApiStage.ASSESSMENT.value, + "stage_status": "COMPLETED", + "stage_batches": {}, + "stage_output_urls": {}, + "verdicts": {}, + "counters": {}, + "gate_passed": [True], + "provider": "openai", + "model": "gpt-4o", + "input_schema": None, + "callback_url": "", + "request_metadata": None, + } + bag.update(overrides) + return bag # type: ignore[return-value] + + +def _seed(db, auth, *, rows: int = 1): + assessment = api.create_assessment( + session=db, + method=AssessmentMethod.BATCH, + input={"data": [{"a": str(i)} for i in range(rows)]}, + organization_id=auth.organization_id, + project_id=auth.project_id, + ) + config = create_test_config( + db, + project_id=auth.project_id, + name=f"assess-{random_lower_string()}", + config_blob=_BLOB, + tag=ConfigTag.ASSESSMENT, + ) + execution = api.create_execution( + session=db, + assessment_id=assessment.id, + config_id=config.id, + config_version=1, + total_items=rows, + ) + return assessment, execution + + +def _batch_job(db, auth, **kwargs) -> BatchJob: + job = BatchJob( + provider="openai", + job_type=BatchJobType.ASSESSMENT.value, + organization_id=auth.organization_id, + project_id=auth.project_id, + total_items=1, + **kwargs, + ) + db.add(job) + db.commit() + db.refresh(job) + return job + + +class _Uploads: + """Records every errors.jsonl upload instead of writing to object storage.""" + + def __init__(self, url: str | None = "s3://bucket/errors.jsonl") -> None: + self.url = url + self.calls: list[dict] = [] + + def __call__(self, *, storage, results, filename, subdirectory) -> str | None: + self.calls.append( + { + "rows": list(results), + "filename": filename, + "subdirectory": subdirectory, + } + ) + return self.url + + @property + def rows(self) -> list[dict]: + return self.calls[-1]["rows"] + + +def _storage_patch(storage: MagicMock | None = None): + return patch( + "app.services.assessment.api.result_files.get_cloud_storage", + return_value=storage or MagicMock(), + ) + + +def _upload_patch(uploads: _Uploads): + return patch( + "app.services.assessment.api.result_files.upload_jsonl_to_object_store", + new=uploads, + ) + + +class TestStageFileKind: + def test_assessment_stage_is_the_runs_results(self) -> None: + assert stage_file_kind(ApiStage.ASSESSMENT.value) == "results" + + def test_prefilter_stage_is_suffixed(self) -> None: + assert stage_file_kind(ApiStage.TOPIC_RELEVANCE.value) == ( + "topic_relevance_results" + ) + + +class TestRecordStageDump: + def test_dump_is_on_the_parent_row_before_any_terminal_state(self, db) -> None: + auth = get_user_test_auth_context(db) + assessment, _ = _seed(db, auth) + + record_stage_dump( + session=db, + assessment=assessment, + stage=ApiStage.TOPIC_RELEVANCE.value, + url="s3://bucket/batch-1170/output.jsonl", + count=998, + ) + + db.refresh(assessment) + assert assessment.result_files == { + "topic_relevance_results": { + "url": "s3://bucket/batch-1170/output.jsonl", + "count": 998, + } + } + + def test_missing_url_records_nothing(self, db) -> None: + auth = get_user_test_auth_context(db) + assessment, _ = _seed(db, auth) + + record_stage_dump( + session=db, + assessment=assessment, + stage=ApiStage.ASSESSMENT.value, + url=None, + count=0, + ) + + db.refresh(assessment) + assert assessment.result_files == {} + + +class TestBuildAndUploadErrors: + def test_clean_success_still_uploads_an_empty_file(self, db) -> None: + auth = get_user_test_auth_context(db) + assessment, execution = _seed(db, auth) + uploads = _Uploads() + + with _storage_patch(), _upload_patch(uploads): + url, count = build_and_upload_errors( + session=db, + execution=execution, + assessment=assessment, + bag=_bag(), + failure_message=None, + ) + + assert (url, count) == ("s3://bucket/errors.jsonl", 0) + assert uploads.rows == [] + assert uploads.calls[0]["filename"] == "errors.jsonl" + assert ( + uploads.calls[0]["subdirectory"] == f"assessment/execution-{execution.id}" + ) + + def test_row_errors_are_flattened_per_stage(self, db) -> None: + auth = get_user_test_auth_context(db) + assessment, execution = _seed(db, auth, rows=2) + uploads = _Uploads() + bag = _bag( + stage_errors={ApiStage.ASSESSMENT.value: {"1": "rate limit exceeded"}} + ) + + with _storage_patch(), _upload_patch(uploads): + _, count = build_and_upload_errors( + session=db, + execution=execution, + assessment=assessment, + bag=bag, + failure_message=None, + ) + + assert count == 1 + assert uploads.rows == [ + { + "type": "row_error", + "stage": ApiStage.ASSESSMENT.value, + "row_index": 1, + "error": "rate limit exceeded", + } + ] + + def test_openai_error_file_lines_become_rows(self, db) -> None: + auth = get_user_test_auth_context(db) + assessment, execution = _seed(db, auth) + job = _batch_job(db, auth, provider_error_file_id="file-err-1") + uploads = _Uploads() + error_file = ( + json.dumps({"custom_id": "row_0", "error": {"message": "bad request"}}) + + "\n" + + json.dumps({"custom_id": "row_3", "error": {"message": "too long"}}) + + "\n" + ) + provider = MagicMock() + provider.download_file.return_value = error_file + + with ( + _storage_patch(), + _upload_patch(uploads), + patch( + "app.services.assessment.api.result_files._build_batch_provider", + return_value=provider, + ), + ): + _, count = build_and_upload_errors( + session=db, + execution=execution, + assessment=assessment, + bag=_bag(stage_batches={ApiStage.ASSESSMENT.value: job.id}), + failure_message=None, + ) + + assert count == 2 + assert [row["type"] for row in uploads.rows] == [ + "provider_error_file", + "provider_error_file", + ] + assert [row["entry"]["custom_id"] for row in uploads.rows] == [ + "row_0", + "row_3", + ] + assert uploads.rows[0]["provider_error_file_id"] == "file-err-1" + provider.download_file.assert_called_once_with("file-err-1") + + def test_unreadable_error_file_degrades_to_one_row(self, db) -> None: + auth = get_user_test_auth_context(db) + assessment, execution = _seed(db, auth) + job = _batch_job(db, auth, provider_error_file_id="file-err-2") + uploads = _Uploads() + provider = MagicMock() + provider.download_file.side_effect = RuntimeError("404 file expired") + + with ( + _storage_patch(), + _upload_patch(uploads), + patch( + "app.services.assessment.api.result_files._build_batch_provider", + return_value=provider, + ), + ): + _, count = build_and_upload_errors( + session=db, + execution=execution, + assessment=assessment, + bag=_bag(stage_batches={ApiStage.ASSESSMENT.value: job.id}), + failure_message=None, + ) + + assert count == 1 + row = uploads.rows[0] + assert row["type"] == "provider_error_file_unavailable" + assert row["provider_error_file_id"] == "file-err-2" + assert "404 file expired" in row["error"] + + def test_batch_without_an_error_file_contributes_nothing(self, db) -> None: + auth = get_user_test_auth_context(db) + assessment, execution = _seed(db, auth) + job = _batch_job(db, auth) # Anthropic/Gemini report errors inline, no file id + uploads = _Uploads() + + with _storage_patch(), _upload_patch(uploads): + _, count = build_and_upload_errors( + session=db, + execution=execution, + assessment=assessment, + bag=_bag(stage_batches={ApiStage.ASSESSMENT.value: job.id}), + failure_message=None, + ) + + assert count == 0 + + def test_storage_outage_yields_no_url(self, db) -> None: + auth = get_user_test_auth_context(db) + assessment, execution = _seed(db, auth) + + with patch( + "app.services.assessment.api.result_files.get_cloud_storage", + side_effect=RuntimeError("s3 unreachable"), + ): + url, count = build_and_upload_errors( + session=db, + execution=execution, + assessment=assessment, + bag=_bag(), + failure_message="kaboom", + ) + + assert url is None + assert count == 1 + + +class TestFinalizeResultFiles: + def test_pre_provider_failure_records_only_a_synthetic_execution_error( + self, db + ) -> None: + auth = get_user_test_auth_context(db) + assessment, execution = _seed(db, auth) + uploads = _Uploads() + + with _storage_patch(), _upload_patch(uploads): + finalize_result_files( + session=db, + execution=execution, + assessment=assessment, + bag=_bag(), + failure_message="Vertex model gemini-2.5-pro not found", + ) + + db.refresh(assessment) + assert set(assessment.result_files) == {"errors"} + assert assessment.result_files["errors"] == { + "url": "s3://bucket/errors.jsonl", + "count": 1, + } + assert uploads.rows == [ + { + "type": "execution_error", + "stage": ApiStage.ASSESSMENT.value, + "error": "Vertex model gemini-2.5-pro not found", + } + ] + + def test_completed_run_carries_both_a_results_and_an_errors_record( + self, db + ) -> None: + auth = get_user_test_auth_context(db) + assessment, execution = _seed(db, auth, rows=2) + record_stage_dump( + session=db, + assessment=assessment, + stage=ApiStage.ASSESSMENT.value, + url="s3://bucket/batch-1173/output.jsonl", + count=2, + ) + uploads = _Uploads() + + with _storage_patch(), _upload_patch(uploads): + finalize_result_files( + session=db, + execution=execution, + assessment=assessment, + bag=_bag( + stage_output_urls={ + ApiStage.ASSESSMENT.value: "s3://bucket/batch-1173/output.jsonl" + } + ), + ) + + db.refresh(assessment) + assert assessment.result_files == { + "results": {"url": "s3://bucket/batch-1173/output.jsonl", "count": 2}, + "errors": {"url": "s3://bucket/errors.jsonl", "count": 0}, + } + + def test_prefilter_and_assessment_dumps_coexist(self, db) -> None: + auth = get_user_test_auth_context(db) + assessment, execution = _seed(db, auth, rows=2) + uploads = _Uploads() + bag = _bag( + stage_output_urls={ + ApiStage.TOPIC_RELEVANCE.value: "s3://bucket/batch-1170/output.jsonl", + ApiStage.ASSESSMENT.value: "s3://bucket/batch-1173/output.jsonl", + }, + counters={ + ApiStage.TOPIC_RELEVANCE.value: { + "total": 2, + "passed": 1, + "rejected": 1, + }, + ApiStage.ASSESSMENT.value: {"total": 1, "passed": 1, "rejected": 0}, + }, + ) + + with _storage_patch(), _upload_patch(uploads): + finalize_result_files( + session=db, execution=execution, assessment=assessment, bag=bag + ) + + db.refresh(assessment) + assert set(assessment.result_files) == { + "topic_relevance_results", + "results", + "errors", + } + assert assessment.result_files["topic_relevance_results"]["count"] == 2 + + def test_a_second_tick_does_not_duplicate_or_lose_records(self, db) -> None: + auth = get_user_test_auth_context(db) + assessment, execution = _seed(db, auth) + uploads = _Uploads() + bag = _bag( + stage_output_urls={ + ApiStage.ASSESSMENT.value: "s3://bucket/batch-1173/output.jsonl" + } + ) + + with _storage_patch(), _upload_patch(uploads): + finalize_result_files( + session=db, execution=execution, assessment=assessment, bag=bag + ) + finalize_result_files( + session=db, execution=execution, assessment=assessment, bag=bag + ) + + db.refresh(assessment) + assert set(assessment.result_files) == {"results", "errors"} + + def test_upload_failure_does_not_raise_into_the_terminal_path(self, db) -> None: + auth = get_user_test_auth_context(db) + assessment, execution = _seed(db, auth) + + with ( + _storage_patch(), + patch( + "app.services.assessment.api.result_files." + "upload_jsonl_to_object_store", + side_effect=RuntimeError("bucket write denied"), + ), + ): + finalize_result_files( + session=db, + execution=execution, + assessment=assessment, + bag=_bag( + stage_output_urls={ + ApiStage.ASSESSMENT.value: "s3://bucket/out.jsonl" + } + ), + ) + + db.refresh(assessment) + assert assessment.result_files == {} + + +class TestBuildCallbackMetadata: + def _signing_storage(self, failing_url: str | None = None) -> MagicMock: + storage = MagicMock() + + def sign(url, expires_in): + if url == failing_url: + raise RuntimeError("presign refused") + return f"https://signed.example/{url}?exp={expires_in}" + + storage.get_signed_url.side_effect = sign + return storage + + def test_every_kind_is_signed_and_keeps_its_count(self, db) -> None: + auth = get_user_test_auth_context(db) + assessment, _ = _seed(db, auth) + api.set_result_files( + session=db, + assessment=assessment, + files={ + "results": {"url": "s3://bucket/out.jsonl", "count": 998}, + "errors": {"url": "s3://bucket/errors.jsonl", "count": 389}, + }, + ) + + with _storage_patch(self._signing_storage()): + metadata = build_callback_metadata(session=db, assessment=assessment) + + assert metadata["result_files"]["results"] == { + "url": f"https://signed.example/s3://bucket/out.jsonl?exp={ONE_DAY_SECONDS}", + "count": 998, + } + assert metadata["result_files"]["errors"]["count"] == 389 + + def test_expires_at_is_one_day_out(self, db) -> None: + auth = get_user_test_auth_context(db) + assessment, _ = _seed(db, auth) + + with _storage_patch(self._signing_storage()): + metadata = build_callback_metadata(session=db, assessment=assessment) + + expires_at = datetime.fromisoformat(metadata["expires_at"]) + assert timedelta(hours=23, minutes=59) < expires_at - now() <= timedelta(days=1) + + def test_a_failing_presign_drops_only_its_own_kind(self, db) -> None: + auth = get_user_test_auth_context(db) + assessment, _ = _seed(db, auth) + api.set_result_files( + session=db, + assessment=assessment, + files={ + "results": {"url": "s3://bucket/out.jsonl", "count": 998}, + "errors": {"url": "s3://bucket/errors.jsonl", "count": 389}, + }, + ) + + with _storage_patch(self._signing_storage(failing_url="s3://bucket/out.jsonl")): + metadata = build_callback_metadata(session=db, assessment=assessment) + + assert set(metadata["result_files"]) == {"errors"} + assert metadata["expires_at"] + + def test_storage_outage_still_returns_the_envelope_keys(self, db) -> None: + auth = get_user_test_auth_context(db) + assessment, _ = _seed(db, auth) + api.set_result_files( + session=db, + assessment=assessment, + files={"results": {"url": "s3://bucket/out.jsonl", "count": 1}}, + ) + + with patch( + "app.services.assessment.api.result_files.get_cloud_storage", + side_effect=RuntimeError("s3 unreachable"), + ): + metadata = build_callback_metadata(session=db, assessment=assessment) + + assert metadata["result_files"] == {} + assert metadata["expires_at"] diff --git a/backend/app/tests/core/batch/test_polling.py b/backend/app/tests/core/batch/test_polling.py new file mode 100644 index 000000000..4b40916be --- /dev/null +++ b/backend/app/tests/core/batch/test_polling.py @@ -0,0 +1,128 @@ +"""Tests for poll_batch_status (app/core/batch/polling.py). + +Real batch_job rows on the transactional session; only the provider client is stubbed. +""" + +from unittest.mock import MagicMock + +from sqlmodel import Session + +from app.core.batch.polling import poll_batch_status +from app.models.batch_job import BatchJob, BatchJobType +from app.tests.utils.auth import get_user_test_auth_context + + +def _batch_job(db: Session, **kwargs) -> BatchJob: + auth = get_user_test_auth_context(db) + job = BatchJob( + provider="openai", + job_type=BatchJobType.ASSESSMENT.value, + organization_id=auth.organization_id, + project_id=auth.project_id, + provider_batch_id="batch_abc", + total_items=1, + **kwargs, + ) + db.add(job) + db.commit() + db.refresh(job) + return job + + +def _provider(status_result: dict) -> MagicMock: + provider = MagicMock() + provider.get_batch_status.return_value = status_result + return provider + + +class TestPollBatchStatus: + def test_error_file_id_persists_when_the_status_did_not_change( + self, db: Session + ) -> None: + job = _batch_job(db, provider_status="in_progress") + + poll_batch_status( + session=db, + provider=_provider( + {"provider_status": "in_progress", "error_file_id": "file-err-1"} + ), + batch_job=job, + ) + + db.refresh(job) + assert job.provider_error_file_id == "file-err-1" + assert job.provider_status == "in_progress" + + def test_unchanged_fields_are_not_written(self, db: Session) -> None: + job = _batch_job( + db, + provider_status="completed", + provider_output_file_id="file-out", + provider_error_file_id="file-err", + ) + before = job.updated_at + + poll_batch_status( + session=db, + provider=_provider( + { + "provider_status": "completed", + "provider_output_file_id": "file-out", + "error_file_id": "file-err", + } + ), + batch_job=job, + ) + + db.refresh(job) + assert job.updated_at == before + + def test_omitted_field_does_not_null_the_stored_value(self, db: Session) -> None: + job = _batch_job( + db, provider_status="in_progress", provider_output_file_id="file-out" + ) + + poll_batch_status( + session=db, + provider=_provider({"provider_status": "completed"}), + batch_job=job, + ) + + db.refresh(job) + assert job.provider_status == "completed" + assert job.provider_output_file_id == "file-out" + + def test_status_flip_persists_the_terminal_fields(self, db: Session) -> None: + job = _batch_job(db, provider_status="in_progress") + + poll_batch_status( + session=db, + provider=_provider( + { + "provider_status": "failed", + "provider_output_file_id": "file-out", + "error_message": "provider rejected the batch", + } + ), + batch_job=job, + ) + + db.refresh(job) + assert job.provider_status == "failed" + assert job.provider_output_file_id == "file-out" + assert job.error_message == "provider rejected the batch" + + def test_status_result_is_returned_verbatim(self, db: Session) -> None: + # _poll_outcome reads error_file_id off the return value, not off the row. + job = _batch_job(db, provider_status="in_progress") + status_result = { + "provider_status": "completed", + "error_file_id": "file-err-2", + "request_counts": {"completed": 3, "failed": 1}, + } + + returned = poll_batch_status( + session=db, provider=_provider(status_result), batch_job=job + ) + + assert returned == status_result diff --git a/backend/app/tests/services/llm/test_mappers.py b/backend/app/tests/services/llm/test_mappers.py index 77a862788..97bd5a04e 100644 --- a/backend/app/tests/services/llm/test_mappers.py +++ b/backend/app/tests/services/llm/test_mappers.py @@ -204,6 +204,28 @@ def test_text_completion_max_num_results_unsupported(self): assert len(warnings) == 1 assert "max_num_results" in warnings[0] + def test_enum_json_schema_carries_only_the_camel_case_ordering_key(self): + """Vertex rejects a payload carrying both ordering spellings; the SDK dump + only emits the snake_case one for some schemas, and an enum triggers it.""" + result, _ = map_kaapi_to_google_params( + { + "model": "gemini-2.5-pro", + "json_schema": { + "type": "object", + "properties": { + "band": {"type": "string", "enum": ["low", "high"]}, + "score": {"type": "integer"}, + }, + "required": ["band", "score"], + }, + }, + completion_type="text", + ) + + google_schema = result["json_schema"] + assert "property_ordering" not in google_schema + assert google_schema["propertyOrdering"] == ["band", "score"] + def test_stt_completion_with_instructions(self): """Test STT completion with instructions parameter.""" kaapi_params = STTLLMParams( diff --git a/docs/wiki/domain-map.md b/docs/wiki/domain-map.md index 7187f0129..073d65540 100644 --- a/docs/wiki/domain-map.md +++ b/docs/wiki/domain-map.md @@ -29,12 +29,13 @@ APIKey → Organization, Project, User # programmatic access | LlmChain | llm/request.py | Org, Project | LlmCall | | Job | job.py | Project | LlmCall; Assessment (RESPONSE method); Celery job execution (logical); evaluation prompt improvement (`JobType.PROMPT_IMPROVEMENT`, logical) | | BatchJob | batch_job.py | Org, Project | EvaluationRun, Assessment; batch polling cron (logical) | -| EvaluationDataset | evaluation.py | Org, Project, Language | EvaluationRun, STTSample (via stt_evaluation), Assessment | +| EvaluationDataset | evaluation.py | Org, Project, Language | EvaluationRun, STTSample (via stt_evaluation) | +| AssessmentSubmission | assessment/submission.py | Org, Project | Assessment (`submission_id`); assessment BATCH `submission_doc_id` requests (logical) | | EvaluationRun | evaluation.py | Dataset, Config, BatchJob, Org, Project, Language | STTResult, TTSResult; Langfuse scores (logical); console UI (logical) | | EvaluationIterationRun | evaluation_iteration.py | Dataset, Config, Org, Project | EvaluationRun, Job (referenced only inside the LangGraph checkpoint state, not FK columns on this table, logical); callback_url caller (logical) | | STTSample / STTResult | stt_evaluation.py | Dataset, Run, File, Language | human annotation UI (logical) | | TTSResult | tts_evaluation.py | Run, Org, Project | human annotation UI (logical) | -| Assessment / AssessmentRun | assessment.py | Config, Dataset, BatchJob, Job, Org, Project | console UI (logical) | +| Assessment / AssessmentRun | assessment.py | Config, AssessmentSubmission, BatchJob, Job, Org, Project | console UI (logical); webhook consumers (logical) | | Document | document.py | Project, Document (parent) | DocumentCollection, DocTransformationJob, FineTuning, ModelEvaluation | | Collection | collection.py | Project | DocumentCollection, CollectionJob; provider vector stores (logical) | | DocumentCollection | document_collection.py | Document, Collection | RAG lookups (logical) | diff --git a/docs/wiki/modules/assessment.md b/docs/wiki/modules/assessment.md index 4314faade..d1e424e89 100644 --- a/docs/wiki/modules/assessment.md +++ b/docs/wiki/modules/assessment.md @@ -9,27 +9,38 @@ All paths relative to `backend/app/`. - `api/routes/assessment/api.py` — API-client route, mounted top-level at `/assessments` (**`POST /assessments` only** — no status/result poll endpoint; the result is delivered by webhook to the request's required `callback_url`); method inferred from input shape. BATCH wired; RESPONSE returns 501 (deferred) ## Tables (SQLModel) -`models/assessment/` is a package, split by surface: `assessment.py` holds the shared DB tables + `AssessmentStatus`/`AssessmentMethod` enums + `AssessmentConfigRef` + the legacy RUN (UI) models; `assessment_api.py` holds the API-client request/response models. Both are re-exported from the package `__init__`, so `from app.models.assessment import X` resolves either. +`models/assessment/` is a package, split by surface: `assessment.py` holds the shared DB tables + `AssessmentStatus`/`AssessmentMethod` enums + `AssessmentConfigRef` + the legacy RUN (UI) models; `assessment_api.py` holds the API-client request/response models; `submission.py` holds the uploaded-submission table and its response models. All are re-exported from the package `__init__`, so `from app.models.assessment import X` resolves either. | Table | Model | |---|---| -| `assessment` (Assessment; parent — `method`, data source; FK → job (RESPONSE), evaluation_dataset, org, project) | `models/assessment/assessment.py` | +| `assessment` (Assessment; parent — `method`, data source, `submission_input`, `result_files`; FK → job (RESPONSE), assessment_submission (RUN), org, project) | `models/assessment/assessment.py` | | `assessment_run` (AssessmentRun; child — one config execution, BATCH/RUN; FK → assessment, config, batch_job) | `models/assessment/assessment.py` | +| `assessment_submission` (AssessmentSubmission; an uploaded CSV/XLSX a run can read its rows from; FK → org, project) | `models/assessment/submission.py` | -Config version (tag=ASSESSMENT, `models/config/assessment_blob.py`) owns system / pre-filters / params / schemas: `input_schema` is a **top-level** field on the blob (sibling of `pre_filters`/`assessment`, **mandatory, non-empty** per-column spec `{type, format}` for the BATCH `data` rows — `type` is required per column; every declared column must be present in every submission row; attachment columns are url-format only) — it describes the shared input rows once, so both the pre-filter and assessment consumers read the same schema. `submission` (the per-row prompt template with `{column}` placeholders — **mandatory** on `assessment.params`, optional on each pre-filter's `params`; every `{placeholder}` is validated at config save against the top-level `input_schema` keys, and an unknown placeholder rejects the save). `json_output_schema` stays in `assessment.params` (assessment-specific, object-typed structured-output schema, omit for free text). The only API-client pre-filter is `topic_relevance` (duplicate_detection was removed from the API-client pipeline; it survives only in the legacy RUN pipeline); it carries its own `provider` (default `openai`) + `params` (TextLLMParams: model, temperature, ...) and runs its own llm call; its criteria live in `params.instructions` (a **mandatory** field, same shape as the assessment call — pre-filters no longer have a top-level `prompt`/`content`) and it may carry its own `params.submission` template. The prompt template lives on the config, not the request. Strict input types `ResponseInput` (RESPONSE, `{attachments}`) / `BatchInput` (BATCH, `{data}` where `data` is a **list of submission rows** — each a flat column→string map, an attachment column's value being a url string) no longer carry `query`; they discriminate structurally on the `data` key (`data` ⇒ BATCH, else RESPONSE) with `extra=forbid` keeping them disjoint and rejecting a stray `query` — no `mode` tag (`models/assessment/assessment_api.py`). Legacy RUN runtime lives in `assessment_run.execution` (`RunExecution`). +Config version (tag=ASSESSMENT, `models/config/assessment_blob.py`) owns system / pre-filters / params / schemas: `input_schema` is a **top-level** field on the blob (sibling of `pre_filters`/`assessment`, **mandatory, non-empty** per-column spec `{type, format}` for the BATCH `data` rows — `type` is required per column; every declared column must be present in every submission row; attachment columns are url-format only) — it describes the shared input rows once, so both the pre-filter and assessment consumers read the same schema. `submission` (the per-row prompt template with `{column}` placeholders — **mandatory** on `assessment.params`, optional on each pre-filter's `params`; every `{placeholder}` is validated at config save against the top-level `input_schema` keys, and an unknown placeholder rejects the save). `json_output_schema` stays in `assessment.params` (assessment-specific, object-typed structured-output schema, omit for free text). The only API-client pre-filter is `topic_relevance` (duplicate_detection was removed from the API-client pipeline; it survives only in the legacy RUN pipeline); it carries its own `provider` (default `openai`) + `params` (TextLLMParams: model, temperature, ...) and runs its own llm call; its criteria live in `params.instructions` (a **mandatory** field, same shape as the assessment call — pre-filters no longer have a top-level `prompt`/`content`) and it may carry its own `params.submission` template. The prompt template lives on the config, not the request. Strict input types `ResponseInput` (RESPONSE, `{attachments}`) / `BatchInput` (BATCH) no longer carry `query`; they discriminate structurally (a `data` or `submission_doc_id` key ⇒ BATCH, else RESPONSE) with `extra=forbid` keeping them disjoint and rejecting a stray `query` — no `mode` tag (`models/assessment/assessment_api.py`). `BatchInput` takes the rows **either** inline as `data` (a list of submission rows, each a flat column→string map, an attachment column's value being a url string) **or** by reference as `submission_doc_id` (an `assessment_submission` id); a `model_validator` requires exactly one, so both-or-neither is a 422. A `submission_doc_id` is resolved at submit: its rows are read, validated against `input_schema` like inline ones, and copied into that run's own `submission.jsonl`, so downstream stays one code path and the run keeps an immutable snapshot if the submission file is later replaced. `assessment.submission_id` records which submission it came from, so the provenance survives the copy and `delete_submission` refuses while a BATCH run still points at it. That column is set by RUN and by a `submission_doc_id` BATCH alike; it stays NULL only when BATCH sent rows inline. Legacy RUN runtime lives in `assessment_run.execution` (`RunExecution`). -`assessment.id` is a **UUID** (like config/job/llm_call). Per-item result = `AssessmentResult {output: {assessment, pre_filter}, error}` (no `metadata` — the provider/model/usage block was removed from the API-client output) where `output.assessment` = the LLM output parsed to an object when the config has a `json_output_schema`, else string (null for gated/failed rows), and `output.pre_filter` holds the `{topic_relevance}` verdict (`{verdict, reasoning}` or null) and is itself null when no pre-filter ran. Delivery is **webhook-only**: the `POST /assessments` ack is the flat `AssessmentSubmitResponse {assessment_id, status, message, inserted_at, updated_at}`, and the result is delivered solely by POSTing the `AssessmentCallback {assessment_id, status, data, request_metadata}` to the request's required `callback_url` on completion — where `data` is a single `AssessmentResult` (RESPONSE) or an `AssessmentBatchResult {total_items, counts, items}` (BATCH); `status` lives on the envelope only. Pre-filter `stop_on_fail` flag (`config/assessment_blob.py`) drives which filters hard-stop the chain on a failing verdict vs pass-through (record only). +`assessment.id` is a **UUID** (like config/job/llm_call). Per-item result = `AssessmentResult {output: {assessment, pre_filter}, error}` (no `metadata` — the provider/model/usage block was removed from the API-client output) where `output.assessment` = the LLM output parsed to an object when the config has a `json_output_schema`, else string (null for gated/failed rows), and `output.pre_filter` holds the `{topic_relevance}` verdict (`{verdict, reasoning}` or null) and is itself null when no pre-filter ran. The API-client BATCH rows do **not** live in `assessment.input` (that column is now RESPONSE/RUN only, NULL for BATCH). They are uploaded to `submission.jsonl` at submit and `assessment.submission_input` holds its `s3://` url: 3-6MB of JSONB was dragged along by every full-row `SELECT` of the assessment. `services/assessment/api/submission_store.py` owns the round trip (`upload_submission_rows` / `load_submission_rows`); rows are fetched only inside `_submit_stage`, never on a poll tick, and a storage read failure raises `SubmissionUnavailableError`, which requeues the tick instead of failing the run. An upload failure at submit is a 503 (an assessment without its rows is unrunnable). `build_result` reads `execution.total_items` rather than re-deriving the count from the rows, so the terminal path never fetches them. + +`assessment.result_files` is the durable record of every provider batch dump held: JSONB, **NOT NULL default `{}`**, CHECK-constrained to a JSON object (`ck_assessment_result_files_is_object`), keyed by file kind (`results` / `errors` / `_results`, derived by `stage_file_kind`) with `{object_store_url}` per kind. It stores the raw `s3://` url (presigning is per-delivery, so the column never holds an expiring link); writes go through `crud/assessment/api.py::set_result_files`, which merges **server-side** (`result_files || :files::jsonb`) rather than read-modify-write, because two drivers can touch the row in the same second. `batch_job.provider_error_file_id` persists OpenAI's `error_file_id` (OpenAI only; Anthropic and Gemini report per-item errors inline) so the error dump is still fetchable at terminal time, in a later tick than the poll that surfaced it. + +Delivery is **webhook-only**: the `POST /assessments` ack is the flat `AssessmentSubmitResponse {assessment_id, status, message, inserted_at, updated_at}`, and the result is delivered solely by POSTing the `AssessmentCallback {assessment_id, status, data, request_metadata}` to the request's required `callback_url` on completion — where `data` is a single `AssessmentResult` (RESPONSE) or an `AssessmentBatchResult {total_items, counts, items}` (BATCH); `status` lives on the envelope only. The outer `send_callback` envelope carries `{success, data, error, metadata}`: `error` is `_fail`'s failure message (null on a success path) and `metadata` is `{result_files: {kind: {signed_url}}, expires_at}` — the column's `object_store_url` presigned for 1 day at delivery time, never stored, so a client whose receiver rejects the (large, item-inlining) body can still fetch the dumps. Both were hardcoded `None` before. Delivery is one inline attempt with no retry, so a rejected callback is still lost. Pre-filter `stop_on_fail` flag (`config/assessment_blob.py`) drives which filters hard-stop the chain on a failing verdict vs pass-through (record only). ## Services / CRUD - `services/assessment/utils/attachments.py` — cell→provider attachment conversion (Drive URL normalization; OpenAI/Anthropic/Gemini part builders). `rewrite_gcs_attachment_urls` bulk-resolves `gs://` cells to provider-reachable URLs via `services/buckets/` (Path A native passthrough for google-gcp / Path B signed HTTPS otherwise) **before** JSONL build. Called in: `services/assessment/api/batch.py::_submit_provider_batch` (API pipeline, all stages), `crud/assessment/batch.py::submit_assessment_batch` (legacy L2), `services/assessment/tasks.py` (legacy prefilter). Submit-time validation (`services/assessment/api/submission.py`) allows `gs://` alongside `http(s)://`. - `services/assessment/` — legacy RUN pipeline (service, stages, processing, batch, cron, tasks) -- `services/assessment/api/` — API-client pipeline: `submission.py` (submit), `batch.py` (staged provider batches — gate pre-filters → pass-through → assessment, over `core/batch`; `PREFILTER_VERDICT_SCHEMA`), `results.py` (builds `AssessmentBatchResult`), `callbacks.py` (webhook) -- `crud/assessment/api.py` — new API-client crud (method-based Assessment/AssessmentRun writes): `create_assessment`, `set_assessment_job`, `create_execution`, `set_execution_batch_job`, `update_status`, `list_executions` (no `get_assessment` — delivery is webhook-only, so there is no request-time fetch). Namespaced under `api` (`from app.crud.assessment import api`) to avoid colliding with the legacy `create_assessment`. +- `services/assessment/submission.py` — submission-file upload + preview (`upload_submission`, `preview_submission`, `file_extension_of`). The name is sanitized then checked via `crud/assessment/submission.py::get_submission_by_name` **before** the object-store write (409 on a duplicate): the key is `assessment/submissions/.` (`SUBMISSIONS_SUBDIRECTORY`) with no timestamp, so a late reject would have overwritten the existing file. No `file_extension` column exists; the format is read off the key's suffix. Row parsing lives in `crud/assessment/batch.py::load_submission_file_rows` (CSV + XLSX), shared by the legacy RUN pipeline and the API-client `submission_doc_id` path. +- **`evaluation_dataset` is no longer used anywhere in the assessment domain.** Assessment rows used to be `type='assessment'` entries in that shared table, which multiplexes four surfaces behind a discriminator, carries eval-only columns (`language_id`, `langfuse_dataset_id`), and whose name uniqueness ignores `type`, so an eval dataset name blocked an assessment one. `assessment.dataset_id` became `assessment.submission_id` (UUID FK) in migration 083. +- `services/assessment/api/` — API-client pipeline: `submission.py` (submit), `submission_store.py` (submission rows in object storage), `batch.py` (staged provider batches — gate pre-filters → pass-through → assessment, over `core/batch`; `PREFILTER_VERDICT_SCHEMA`), `results.py` (builds `AssessmentBatchResult`), `result_files.py` (dump bookkeeping + `errors.jsonl` + presigned callback `metadata`), `callbacks.py` (webhook) +- `result_files.py` — `stage_file_kind` / `record_stage_dump` (called per completed stage from `batch.py`), `build_and_upload_errors` (one self-describing JSONL of `execution_error` / `row_error` / `provider_error_file` records, uploaded **even when empty** so both urls always exist; per-row errors come from the exec bag's `stage_errors`, OpenAI's dump via `provider.download_file`), `finalize_result_files` (called from `_finalize`/`_fail` *before* the callback_url check, so durability never depends on a callback), `build_callback_metadata` (presigns each kind at `expires_in=86400`; never raises). Storage keys land under one per-assessment prefix, `/assessment//` (`assessment_subdirectory`), holding `errors.jsonl` plus `batch-/results.jsonl` per stage; the API path passes that prefix into `process_completed_batch`'s optional `subdirectory` (legacy RUN and evaluations keep the default `/batch-`). The submission rows sit at `/submission.jsonl` and uploaded submission files at `assessment/submissions/.`. +- `crud/assessment/submission.py` — `create_submission`, `get_submission_by_name`, `get_submission_by_id`, `list_submissions`, `delete_submission` (refuses while an assessment still references it) +- `crud/assessment/api.py` — new API-client crud (method-based Assessment/AssessmentRun writes): `create_assessment`, `set_assessment_job`, `set_submission_input`, `create_execution`, `set_execution_batch_job`, `save_execution_state`, `set_result_files`, `update_status`, `list_executions` (no `get_assessment` — delivery is webhook-only, so there is no request-time fetch). Namespaced under `api` (`from app.crud.assessment import api`) to avoid colliding with the legacy `create_assessment`. - `crud/assessment/{core,cron,processing,batch}.py` — legacy RUN pipeline crud. RUN runtime for the dropped columns (`stage`/`stage_status`/`pipeline`/`stage_batches`/`prefilter_total_*`/`object_store_url`) now lives in the `assessment_run.execution` bag via `core._read_exec`/`_write_exec`; `status` is the `AssessmentStatus` enum; the RUN input binding lives on the parent `assessment.input`. `batch.py` builds per-row prompts (`{column}` substitution from the parent `InputBinding.prompt`). ## Async - Rides shared `core/batch/` provider batch infra + cron polling (same as evaluations). -- API-client BATCH: `celery/tasks/job_execution.py::run_assessment_api_batch` drives the staged pipeline (poll → parse verdict/gate → advance/finalize → callback), self-re-enqueuing between stages. Staged state + `callback_url`/`request_metadata` live in the `assessment_run.execution` bag. +- **Two drivers, strictly separated:** the cron poller (`crud/assessment/cron.py::poll_all_pending_assessment_evaluations`) selects `method == RUN` only and owns the legacy pipeline; the API-client BATCH path is driven solely by the Celery self-re-enqueue. The poller used to pick up BATCH runs too and crash on their list-shaped `pipeline`, leaving the run un-finalised and the callback unfired. Its deterministic-error branch (`DETERMINISTIC_ERRORS`) marks a run FAILED on a programming error instead of logging "will retry" forever; bare `Exception` stays the transient path. +- API-client BATCH: `celery/tasks/job_execution.py::run_assessment_api_batch` drives the staged pipeline (poll → parse verdict/gate → advance/finalize → callback), self-re-enqueuing between stages. Staged state + `callback_url`/`request_metadata` live in the `assessment_run.execution` bag (`BatchRunState`), which also holds `stage_errors` (`stage -> {row_index -> error}`, captured at parse time; the raw dumps go to object storage, not the bag). +- `core/batch/polling.py::poll_batch_status` diffs **per field**, not only on a `provider_status` flip, so `error_file_id` and `provider_output_file_id` persist even when the status is unchanged. Shared with the evaluations path; return value unchanged. ## External - Provider Batch APIs, object storage for attachments (incl. `gs://` attachments resolved via `services/buckets/`). diff --git a/docs/wiki/modules/llm-call.md b/docs/wiki/modules/llm-call.md index 9bb07a7ee..87ad1c990 100644 --- a/docs/wiki/modules/llm-call.md +++ b/docs/wiki/modules/llm-call.md @@ -23,6 +23,7 @@ All paths relative to `backend/app/`. - `LLMCallConfig` — one-of: saved reference (`id` + `version`) XOR ad-hoc `blob` (validator-enforced) - `ConfigBlob` — `completion` + optional `prompt_template` (`PromptTemplate.template`, plain string; `{{input}}` interpolation is llm-chain-only) + `input_guardrails`/`output_guardrails` - `CompletionConfig` — discriminated union on `provider`: `KaapiCompletionConfig` (standardized params: `TextLLMParams`/`STTLLMParams`/`TTSLLMParams`), `NativeCompletionConfig` (pass-through), `ProxyCompletionConfig` (client's own endpoint) +- `TextLLMParams` reasoning knobs: `reasoning` + `effort` (OpenAI-style), `thinking` (Anthropic adaptive-thinking container, forwarded as-is) and `thinking_level` (Gemini). A knob must be **declared here** to survive config save — pydantic's default extra policy is ignore, so an undeclared key is dropped silently at validation with no error. - `QueryParams` — per-call input + `ConversationConfig` - `models/guardrails/` — validator config shapes diff --git a/docs/wiki/modules/platform.md b/docs/wiki/modules/platform.md index a41197c24..19d4923b4 100644 --- a/docs/wiki/modules/platform.md +++ b/docs/wiki/modules/platform.md @@ -13,7 +13,7 @@ All paths relative to `backend/app/`. | Credentials | `api/routes/credentials.py` | `credential` (`models/credentials.py`) | `crud/credentials.py`; provider keys per org/project; envelope encryption (KMS-wrapped data key + AES-GCM), prefix-versioned ciphertexts | | Model config | `api/routes/model_config.py` | `model_config` (`models/model_config.py`) | `crud/model_config.py` | | Bucket providers | — | reuses `credential` (`google-gcp`) | `services/buckets/` — global registry + resolver (`providers/`), GCS V4 signed + bulk-signed URLs with a 24h expiry cap (`providers/gcs.py`, cap in `providers/base.py`), attachment path selection + URL resolution (`attachments.py`) | -| Cron | `api/routes/cron.py` | — | triggers batch polling (`crud/evaluations/cron.py`) | +| Cron | `api/routes/cron.py` | — | triggers batch polling (`crud/evaluations/cron.py`, `crud/assessment/cron.py` — the latter polls `method == RUN` assessments only; API-client BATCH runs are driven by their own Celery self-re-enqueue) | | Jobs | — | `job` (`models/job.py`), `batch_job` (`models/batch_job.py`) | `crud/jobs.py`, `crud/job/`, `services/job_monitoring.py` | ## Credential contracts From bf8c3ec0dbea39fa72f96374aacdc24bd05c8218 Mon Sep 17 00:00:00 2001 From: Prashant Vasudevan <71649489+vprashrex@users.noreply.github.com> Date: Thu, 10 Sep 2026 10:23:35 +0530 Subject: [PATCH 2/4] fix(assessment): align tests with the submission rename Follow-up to the rename: test modules still imported the dataset-era symbols and asserted the old result-file shape, so collection failed. --- .../app/tests/assessment/test_api_batch.py | 41 +++- .../tests/assessment/test_api_submission.py | 21 +- backend/app/tests/assessment/test_batch.py | 64 +++---- backend/app/tests/assessment/test_cron.py | 21 +- backend/app/tests/assessment/test_crud.py | 29 ++- backend/app/tests/assessment/test_export.py | 14 +- .../assessment/test_prefilter_batching.py | 25 ++- .../app/tests/assessment/test_result_files.py | 58 ++---- backend/app/tests/assessment/test_routes.py | 62 +++--- backend/app/tests/assessment/test_service.py | 50 +++-- .../{test_dataset.py => test_submission.py} | 179 +++++++++++------- 11 files changed, 330 insertions(+), 234 deletions(-) rename backend/app/tests/assessment/{test_dataset.py => test_submission.py} (58%) diff --git a/backend/app/tests/assessment/test_api_batch.py b/backend/app/tests/assessment/test_api_batch.py index 208077b92..e1cb0884f 100644 --- a/backend/app/tests/assessment/test_api_batch.py +++ b/backend/app/tests/assessment/test_api_batch.py @@ -6,6 +6,7 @@ """ import json +from uuid import uuid4 from unittest.mock import MagicMock, patch import pytest @@ -68,6 +69,26 @@ TOPIC_CRITERIA = "Is this on topic?" +_SEEDED_ROWS: dict = {} + + +@pytest.fixture(autouse=True) +def _serve_seeded_submission_rows(): + """Submission rows live in object storage; serve the seeded ones instead.""" + + def _load(*, session, assessment): + return BatchInput(data=_SEEDED_ROWS[assessment.id]) + + with patch( + "app.services.assessment.api.submission_store.load_submission_rows", _load + ): + yield + + +def _register_rows(assessment, data) -> None: + _SEEDED_ROWS[assessment.id] = data + + def _blob_dict( *, topic_relevance: bool | None = None, @@ -524,14 +545,14 @@ def _make_batch_job(db, *, org_id, project_id, **kwargs) -> BatchJob: def _seed_assessment(db, *, org_id, project_id, config_id, bag, data, status=None): - batch_input = BatchInput(data=data) assessment = api.create_assessment( session=db, method=AssessmentMethod.BATCH, - input=batch_input.model_dump(mode="json"), + input=None, organization_id=org_id, project_id=project_id, ) + _register_rows(assessment, data) execution = api.create_execution( session=db, assessment_id=assessment.id, @@ -813,7 +834,7 @@ def test_processing_when_status_pending(self, db) -> None: "app.services.assessment.api.batch.poll_batch_status", return_value={}, ): - outcome, results = _poll_outcome(db, self._provider(), job) + outcome, results = _poll_outcome(db, self._provider(), job, uuid4()) assert outcome == "processing" assert results is None @@ -829,7 +850,7 @@ def test_failed_status(self, db) -> None: "app.services.assessment.api.batch.poll_batch_status", return_value={}, ): - outcome, _ = _poll_outcome(db, self._provider(), job) + outcome, _ = _poll_outcome(db, self._provider(), job, uuid4()) assert outcome == "failed" def test_success_but_all_failed_counts(self, db) -> None: @@ -845,7 +866,7 @@ def test_success_but_all_failed_counts(self, db) -> None: "app.services.assessment.api.batch.poll_batch_status", return_value={"request_counts": {"completed": 0, "failed": 3}}, ): - outcome, _ = _poll_outcome(db, self._provider(), job) + outcome, _ = _poll_outcome(db, self._provider(), job, uuid4()) assert outcome == "failed" def test_success_output_not_ready(self, db) -> None: @@ -860,7 +881,7 @@ def test_success_output_not_ready(self, db) -> None: "app.services.assessment.api.batch.poll_batch_status", return_value={"request_counts": {"completed": 1}}, ): - outcome, _ = _poll_outcome(db, self._provider(), job) + outcome, _ = _poll_outcome(db, self._provider(), job, uuid4()) assert outcome == "processing" @@ -1073,10 +1094,11 @@ def test_all_gated_out_skips_submit(self, db) -> None: assessment = api.create_assessment( session=db, method=AssessmentMethod.BATCH, - input=batch_input.model_dump(mode="json"), + input=None, organization_id=auth.organization_id, project_id=auth.project_id, ) + _register_rows(assessment, batch_input.data) execution = api.create_execution( session=db, assessment_id=assessment.id, @@ -1087,8 +1109,8 @@ def test_all_gated_out_skips_submit(self, db) -> None: ok = _submit_stage( session=db, execution=execution, + assessment=assessment, blob=blob, - batch_input=batch_input, bag=bag, stage=ApiStage.ASSESSMENT.value, organization_id=auth.organization_id, @@ -1752,10 +1774,11 @@ def _seed(self, db, auth, *, data, bag): assessment = api.create_assessment( session=db, method=AssessmentMethod.BATCH, - input=batch_input.model_dump(mode="json"), + input=None, organization_id=auth.organization_id, project_id=auth.project_id, ) + _register_rows(assessment, batch_input.data) execution = api.create_execution( session=db, assessment_id=assessment.id, diff --git a/backend/app/tests/assessment/test_api_submission.py b/backend/app/tests/assessment/test_api_submission.py index 2ddbb5854..554617009 100644 --- a/backend/app/tests/assessment/test_api_submission.py +++ b/backend/app/tests/assessment/test_api_submission.py @@ -71,6 +71,15 @@ def _bypass_callback_check(self): with patch("app.services.assessment.api.submission.validate_callback_url"): yield + @pytest.fixture(autouse=True) + def _stub_submission_upload(self): + # Submit stores the rows in object storage; these cases are not about that. + with patch( + "app.services.assessment.api.submission.upload_submission_rows", + return_value="s3://bucket/submission.jsonl", + ): + yield + def test_creates_assessment_run_and_dispatches(self, db) -> None: auth = get_user_test_auth_context(db) config = _assessment_config(db, auth.project_id) @@ -319,6 +328,10 @@ def test_valid_public_https_callback_succeeds(self, db) -> None: "app.services.assessment.api.submission.validate_callback_url" ) as validate, patch("app.celery.tasks.job_execution.run_assessment_api_batch") as task, + patch( + "app.services.assessment.api.submission.upload_submission_rows", + return_value="s3://bucket/submission.jsonl", + ), ): response = self._submit(db, auth, config, "https://example.com/hook") validate.assert_called_once_with("https://example.com/hook") @@ -329,7 +342,13 @@ def test_valid_public_https_callback_succeeds(self, db) -> None: class TestCreateAssessmentRoute: @pytest.fixture(autouse=True) def _bypass_callback_check(self): - with patch("app.services.assessment.api.submission.validate_callback_url"): + with ( + patch("app.services.assessment.api.submission.validate_callback_url"), + patch( + "app.services.assessment.api.submission.upload_submission_rows", + return_value="s3://bucket/submission.jsonl", + ), + ): yield def test_batch_input_dispatches_and_returns_202_body( diff --git a/backend/app/tests/assessment/test_batch.py b/backend/app/tests/assessment/test_batch.py index d00ba8af5..72754e348 100644 --- a/backend/app/tests/assessment/test_batch.py +++ b/backend/app/tests/assessment/test_batch.py @@ -10,7 +10,7 @@ from app.crud.assessment.batch import ( _build_text_prompt, - _load_dataset_rows, + load_submission_file_rows, _parse_excel_rows, build_anthropic_jsonl, build_google_jsonl, @@ -103,17 +103,17 @@ def _make_assessment() -> MagicMock: return assessment -def _make_dataset() -> MagicMock: - dataset = MagicMock() - dataset.id = 8 - return dataset +def _make_submission() -> MagicMock: + submission = MagicMock() + submission.id = 8 + return submission class TestSubmitAssessmentBatchProviderRouting: def test_openai_native_routes_to_openai_batch(self) -> None: session = MagicMock() run = _make_run() - dataset = _make_dataset() + submission = _make_submission() config_blob = SimpleNamespace( completion=SimpleNamespace( provider="openai-native", @@ -126,7 +126,7 @@ def test_openai_native_routes_to_openai_batch(self) -> None: with ( patch( - "app.crud.assessment.batch._load_dataset_rows", + "app.crud.assessment.batch.load_submission_file_rows", return_value=[{"question": "q1"}], ), patch( @@ -154,7 +154,7 @@ def test_openai_native_routes_to_openai_batch(self) -> None: session=session, run=run, assessment=_make_assessment(), - dataset=dataset, + submission=submission, config_blob=config_blob, assessment_input={ "text_columns": ["question"], @@ -177,7 +177,7 @@ def test_openai_native_routes_to_openai_batch(self) -> None: def test_config_instruction_is_used(self) -> None: session = MagicMock() run = _make_run() - dataset = _make_dataset() + submission = _make_submission() config_blob = SimpleNamespace( completion=SimpleNamespace( provider="openai", @@ -190,7 +190,7 @@ def test_config_instruction_is_used(self) -> None: with ( patch( - "app.crud.assessment.batch._load_dataset_rows", + "app.crud.assessment.batch.load_submission_file_rows", return_value=[{"question": "q1"}], ), patch( @@ -218,7 +218,7 @@ def test_config_instruction_is_used(self) -> None: session=session, run=run, assessment=_make_assessment(), - dataset=dataset, + submission=submission, config_blob=config_blob, assessment_input={"text_columns": ["question"], "attachments": []}, organization_id=1, @@ -234,7 +234,7 @@ def test_config_instruction_is_used(self) -> None: def test_google_native_routes_to_google_batch(self) -> None: session = MagicMock() run = _make_run() - dataset = _make_dataset() + submission = _make_submission() config_blob = SimpleNamespace( completion=SimpleNamespace( provider="google-native", @@ -249,7 +249,7 @@ def test_google_native_routes_to_google_batch(self) -> None: with ( patch( - "app.crud.assessment.batch._load_dataset_rows", + "app.crud.assessment.batch.load_submission_file_rows", return_value=[{"question": "q1"}], ), patch( @@ -275,7 +275,7 @@ def test_google_native_routes_to_google_batch(self) -> None: session=session, run=run, assessment=_make_assessment(), - dataset=dataset, + submission=submission, config_blob=config_blob, assessment_input={ "text_columns": ["question"], @@ -293,7 +293,7 @@ def test_google_native_routes_to_google_batch(self) -> None: def test_anthropic_native_routes_to_anthropic_batch(self) -> None: session = MagicMock() run = _make_run() - dataset = _make_dataset() + submission = _make_submission() config_blob = SimpleNamespace( completion=SimpleNamespace( provider="anthropic-native", @@ -306,7 +306,7 @@ def test_anthropic_native_routes_to_anthropic_batch(self) -> None: with ( patch( - "app.crud.assessment.batch._load_dataset_rows", + "app.crud.assessment.batch.load_submission_file_rows", return_value=[{"question": "q1"}], ), patch( @@ -334,7 +334,7 @@ def test_anthropic_native_routes_to_anthropic_batch(self) -> None: session=session, run=run, assessment=_make_assessment(), - dataset=dataset, + submission=submission, config_blob=config_blob, assessment_input={ "text_columns": ["question"], @@ -355,13 +355,12 @@ def test_anthropic_native_routes_to_anthropic_batch(self) -> None: class TestBatchDatasetParsing: - def test_load_dataset_rows_routes_xlsx_to_excel_parser(self) -> None: + def test_load_submission_rows_routes_xlsx_to_excel_parser(self) -> None: session = MagicMock() - dataset = MagicMock() - dataset.id = 8 - dataset.project_id = 1 - dataset.object_store_url = "s3://bucket/key" - dataset.dataset_metadata = {"file_extension": ".xlsx"} + submission = MagicMock() + submission.id = 8 + submission.project_id = 1 + submission.object_store_url = "s3://bucket/key.xlsx" storage = MagicMock() stream_body = MagicMock() @@ -376,18 +375,17 @@ def test_load_dataset_rows_routes_xlsx_to_excel_parser(self) -> None: return_value=expected, ) as parse_excel, ): - result = _load_dataset_rows(session=session, dataset=dataset) + result = load_submission_file_rows(session=session, submission=submission) assert result == expected parse_excel.assert_called_once_with(b"xlsx-content") - def test_load_dataset_rows_rejects_legacy_xls(self) -> None: + def test_load_submission_rows_rejects_legacy_xls(self) -> None: session = MagicMock() - dataset = MagicMock() - dataset.id = 8 - dataset.project_id = 1 - dataset.object_store_url = "s3://bucket/key" - dataset.dataset_metadata = {"file_extension": ".xls"} + submission = MagicMock() + submission.id = 8 + submission.project_id = 1 + submission.object_store_url = "s3://bucket/key.xls" storage = MagicMock() stream_body = MagicMock() @@ -396,7 +394,7 @@ def test_load_dataset_rows_rejects_legacy_xls(self) -> None: with patch("app.crud.assessment.batch.get_cloud_storage", return_value=storage): with pytest.raises(ValueError, match="Legacy Excel format"): - _load_dataset_rows(session=session, dataset=dataset) + load_submission_file_rows(session=session, submission=submission) def test_parse_excel_rows_invalid_payload_raises(self) -> None: with pytest.raises((ValueError, InvalidFileException)): @@ -449,7 +447,9 @@ def test_parse_excel_rows_unexpected_exception_raises_value_error(self) -> None: "app.crud.assessment.batch.openpyxl.load_workbook", side_effect=RuntimeError("boom"), ): - with pytest.raises(ValueError, match="Failed to parse XLSX dataset rows"): + with pytest.raises( + ValueError, match="Failed to parse XLSX submission rows" + ): _parse_excel_rows(b"bad") diff --git a/backend/app/tests/assessment/test_cron.py b/backend/app/tests/assessment/test_cron.py index 38cdedc5c..6eab9496c 100644 --- a/backend/app/tests/assessment/test_cron.py +++ b/backend/app/tests/assessment/test_cron.py @@ -1,6 +1,7 @@ """Tests for assessment/cron.py helper functions.""" from datetime import datetime +from uuid import uuid4 from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -11,7 +12,12 @@ _log_config_progress, poll_all_pending_assessment_evaluations, ) -from app.models.assessment import AssessmentMethod, AssessmentStatus, StageStatus +from app.models.assessment import ( + AssessmentMethod, + AssessmentStatus, + AssessmentSubmission, + StageStatus, +) from app.models.config.assessment_blob import AssessmentConfigBlob from app.models.config.config import ConfigTag from app.tests.utils.auth import get_user_test_auth_context @@ -244,13 +250,20 @@ class TestPollerAgainstRealRows: """The poller's method boundary and its failure classification, on real rows.""" def _run_assessment(self, db, auth): - dataset = create_test_evaluation_dataset( - db, organization_id=auth.organization_id, project_id=auth.project_id + submission = AssessmentSubmission( + name=f"sub-{uuid4().hex[:8]}", + object_store_url="s3://bucket/sub.csv", + total_items=1, + organization_id=auth.organization_id, + project_id=auth.project_id, ) + db.add(submission) + db.commit() + db.refresh(submission) return assessment_core.create_assessment( session=db, experiment_name="exp", - dataset_id=dataset.id, + submission_id=submission.id, organization_id=auth.organization_id, project_id=auth.project_id, ) diff --git a/backend/app/tests/assessment/test_crud.py b/backend/app/tests/assessment/test_crud.py index 961fc37b3..91d5b69af 100644 --- a/backend/app/tests/assessment/test_crud.py +++ b/backend/app/tests/assessment/test_crud.py @@ -13,12 +13,12 @@ build_run_stats, compute_run_counts, create_assessment, - create_assessment_dataset, + create_submission, create_assessment_run, derive_aggregate_error, derive_assessment_status, get_assessment_by_id, - get_assessment_dataset_by_id, + get_submission_by_id, get_assessment_run_by_id, get_assessment_runs_for_assessment, list_assessment_runs, @@ -28,7 +28,6 @@ update_run_post_processing_config, ) from app.crud.assessment.core import update_assessment_run_prefilter_stats -from app.models.stt_evaluation import EvaluationType def _counts(total=0, pending=0, processing=0, completed=0, failed=0): @@ -84,18 +83,17 @@ def test_get_assessment_run_by_id_not_found(self) -> None: assert exc_info.value.status_code == 404 assert "99" in exc_info.value.detail - def test_get_assessment_dataset_by_id_not_found(self) -> None: + def test_get_submission_by_id_not_found(self) -> None: session = MagicMock() session.exec.return_value.first.return_value = None with pytest.raises(HTTPException) as exc_info: - get_assessment_dataset_by_id( + get_submission_by_id( session=session, - dataset_id=99, + submission_id=UUID("00000000-0000-0000-0000-000000000099"), organization_id=1, project_id=1, ) assert exc_info.value.status_code == 404 - assert "99" in exc_info.value.detail def test_get_assessment_runs_for_assessment(self) -> None: session = MagicMock() @@ -104,21 +102,20 @@ def test_get_assessment_runs_for_assessment(self) -> None: class TestCrudWrites: - def test_create_assessment_dataset_uses_assessment_type(self) -> None: + def test_create_submission_persists_row(self) -> None: session = MagicMock() - result = create_assessment_dataset( + result = create_submission( session=session, - name="dataset", + name="submission", description="desc", - dataset_metadata={"total_items_count": 2}, - object_store_url="s3://datasets/file.csv", - langfuse_dataset_id="langfuse-dataset", + object_store_url="s3://submissions/file.csv", + total_items=2, organization_id=1, project_id=1, ) - assert result.type == EvaluationType.ASSESSMENT.value - assert result.langfuse_dataset_id == "langfuse-dataset" + assert result.name == "submission" + assert result.total_items == 2 session.add.assert_called_once() session.commit.assert_called_once() session.refresh.assert_called_once() @@ -128,7 +125,7 @@ def test_create_assessment_success(self) -> None: result = create_assessment( session=session, experiment_name="exp", - dataset_id=1, + submission_id=UUID(int=1), organization_id=1, project_id=1, ) diff --git a/backend/app/tests/assessment/test_export.py b/backend/app/tests/assessment/test_export.py index 9ac64116c..4e443d95a 100644 --- a/backend/app/tests/assessment/test_export.py +++ b/backend/app/tests/assessment/test_export.py @@ -13,7 +13,7 @@ _drop_empty_columns, _expand_input_columns, _expand_output_columns, - _load_dataset_rows_for_run, + _load_submission_rows_for_run, _load_l2_results_for_run, _load_parsed_results_for_batch_job, _load_parsed_results_for_run, @@ -567,7 +567,7 @@ def _make_assessment(self, dataset_id: int = 1) -> MagicMock: def test_dataset_not_found_returns_empty(self) -> None: session = MagicMock() session.get.return_value = None - result = _load_dataset_rows_for_run( + result = _load_submission_rows_for_run( session=session, run=self._make_run(), assessment=self._make_assessment() ) assert result == [] @@ -577,7 +577,7 @@ def test_dataset_no_url_returns_empty(self) -> None: dataset = MagicMock() dataset.object_store_url = None session.get.return_value = dataset - result = _load_dataset_rows_for_run( + result = _load_submission_rows_for_run( session=session, run=self._make_run(), assessment=self._make_assessment() ) assert result == [] @@ -585,7 +585,7 @@ def test_dataset_no_url_returns_empty(self) -> None: def test_exception_returns_empty(self) -> None: session = MagicMock() session.get.side_effect = Exception("DB error") - result = _load_dataset_rows_for_run( + result = _load_submission_rows_for_run( session=session, run=self._make_run(), assessment=self._make_assessment() ) assert result == [] @@ -596,10 +596,10 @@ def test_valid_dataset_returns_rows(self) -> None: dataset.object_store_url = "s3://bucket/ds.csv" session.get.return_value = dataset with patch( - "app.services.assessment.utils.export._load_dataset_rows", + "app.services.assessment.utils.export._load_submission_rows", return_value=[{"q": "hi"}], ): - result = _load_dataset_rows_for_run( + result = _load_submission_rows_for_run( session=session, run=self._make_run(), assessment=self._make_assessment(), @@ -637,7 +637,7 @@ def _patches(self, *, l2, prefilter=None, dataset_rows=None): return_value=prefilter or {}, ), patch( - "app.services.assessment.utils.export._load_dataset_rows_for_run", + "app.services.assessment.utils.export._load_submission_rows_for_run", return_value=dataset_rows if dataset_rows is not None else [], ), ] diff --git a/backend/app/tests/assessment/test_prefilter_batching.py b/backend/app/tests/assessment/test_prefilter_batching.py index a8081491c..367f5f7b4 100644 --- a/backend/app/tests/assessment/test_prefilter_batching.py +++ b/backend/app/tests/assessment/test_prefilter_batching.py @@ -2,6 +2,7 @@ from contextlib import contextmanager from types import SimpleNamespace +from uuid import UUID from unittest.mock import MagicMock, patch import pytest @@ -101,7 +102,9 @@ def _ctx(self, accepted): "_resolve_run_context", return_value=(assessment, MagicMock(), SimpleNamespace(), None), ), - patch.object(tasks, "_load_dataset_rows", return_value=[{"a": "1"}] * 3), + patch.object( + tasks, "load_submission_file_rows", return_value=[{"a": "1"}] * 3 + ), patch.object(tasks, "_accepted_indices", return_value=accepted), patch.object(tasks, "recompute_assessment_status"), ] @@ -160,7 +163,9 @@ def test_prefilter_rewrites_gcs_attachments_before_submit(self) -> None: "_resolve_run_context", return_value=(assessment, MagicMock(), SimpleNamespace(), None), ), - patch.object(tasks, "_load_dataset_rows", return_value=[{"a": "1"}] * 3), + patch.object( + tasks, "load_submission_file_rows", return_value=[{"a": "1"}] * 3 + ), patch.object(tasks, "_accepted_indices", return_value=[0, 1, 2]), patch.object(tasks, "recompute_assessment_status"), patch.object(assessment_core, "flag_modified"), @@ -279,9 +284,9 @@ class TestResolveRunContext: def test_success(self) -> None: session = MagicMock() run = _run() - session.get.return_value = SimpleNamespace(dataset_id=3) + session.get.return_value = SimpleNamespace(submission_id=UUID(int=3)) with patch.object( - tasks, "get_assessment_dataset_by_id", return_value=MagicMock() + tasks, "get_submission_by_id", return_value=MagicMock() ), patch.object( tasks, "resolve_evaluation_config", return_value=({"x": 1}, None) ): @@ -298,9 +303,9 @@ def test_missing_parent(self) -> None: def test_config_error(self) -> None: session = MagicMock() - session.get.return_value = SimpleNamespace(dataset_id=3) + session.get.return_value = SimpleNamespace(submission_id=UUID(int=3)) with patch.object( - tasks, "get_assessment_dataset_by_id", return_value=MagicMock() + tasks, "get_submission_by_id", return_value=MagicMock() ), patch.object( tasks, "resolve_evaluation_config", return_value=(None, "bad config") ): @@ -361,7 +366,9 @@ def test_empty_dataset_fails_run(self) -> None: tasks, "_resolve_run_context", return_value=(SimpleNamespace(), MagicMock(), SimpleNamespace(), None), - ), patch.object(tasks, "_load_dataset_rows", return_value=[]), patch.object( + ), patch.object( + tasks, "load_submission_file_rows", return_value=[] + ), patch.object( assessment_core, "flag_modified" ), patch.object( tasks, "update_assessment_run_status" @@ -389,7 +396,7 @@ def test_submits_l2_batch(self) -> None: None, ), ), patch.object( - tasks, "_load_dataset_rows", return_value=[{"a": "1"}] * 3 + tasks, "load_submission_file_rows", return_value=[{"a": "1"}] * 3 ), patch.object( tasks, "_accepted_indices", return_value=[0, 1] ), patch.object( @@ -415,7 +422,7 @@ def test_unknown_stage_raises(self) -> None: None, ), ), patch.object( - tasks, "_load_dataset_rows", return_value=[{"a": "1"}] + tasks, "load_submission_file_rows", return_value=[{"a": "1"}] ), patch.object( tasks, "_accepted_indices", return_value=[0] ): diff --git a/backend/app/tests/assessment/test_result_files.py b/backend/app/tests/assessment/test_result_files.py index 42676ca09..3a9629a1e 100644 --- a/backend/app/tests/assessment/test_result_files.py +++ b/backend/app/tests/assessment/test_result_files.py @@ -156,14 +156,12 @@ def test_dump_is_on_the_parent_row_before_any_terminal_state(self, db) -> None: assessment=assessment, stage=ApiStage.TOPIC_RELEVANCE.value, url="s3://bucket/batch-1170/output.jsonl", - count=998, ) db.refresh(assessment) assert assessment.result_files == { "topic_relevance_results": { - "url": "s3://bucket/batch-1170/output.jsonl", - "count": 998, + "object_store_url": "s3://bucket/batch-1170/output.jsonl", } } @@ -176,7 +174,6 @@ def test_missing_url_records_nothing(self, db) -> None: assessment=assessment, stage=ApiStage.ASSESSMENT.value, url=None, - count=0, ) db.refresh(assessment) @@ -190,7 +187,7 @@ def test_clean_success_still_uploads_an_empty_file(self, db) -> None: uploads = _Uploads() with _storage_patch(), _upload_patch(uploads): - url, count = build_and_upload_errors( + url = build_and_upload_errors( session=db, execution=execution, assessment=assessment, @@ -198,12 +195,10 @@ def test_clean_success_still_uploads_an_empty_file(self, db) -> None: failure_message=None, ) - assert (url, count) == ("s3://bucket/errors.jsonl", 0) + assert url == "s3://bucket/errors.jsonl" assert uploads.rows == [] assert uploads.calls[0]["filename"] == "errors.jsonl" - assert ( - uploads.calls[0]["subdirectory"] == f"assessment/execution-{execution.id}" - ) + assert uploads.calls[0]["subdirectory"] == f"assessment/{assessment.id}" def test_row_errors_are_flattened_per_stage(self, db) -> None: auth = get_user_test_auth_context(db) @@ -214,15 +209,13 @@ def test_row_errors_are_flattened_per_stage(self, db) -> None: ) with _storage_patch(), _upload_patch(uploads): - _, count = build_and_upload_errors( + build_and_upload_errors( session=db, execution=execution, assessment=assessment, bag=bag, failure_message=None, ) - - assert count == 1 assert uploads.rows == [ { "type": "row_error", @@ -254,15 +247,13 @@ def test_openai_error_file_lines_become_rows(self, db) -> None: return_value=provider, ), ): - _, count = build_and_upload_errors( + build_and_upload_errors( session=db, execution=execution, assessment=assessment, bag=_bag(stage_batches={ApiStage.ASSESSMENT.value: job.id}), failure_message=None, ) - - assert count == 2 assert [row["type"] for row in uploads.rows] == [ "provider_error_file", "provider_error_file", @@ -290,15 +281,13 @@ def test_unreadable_error_file_degrades_to_one_row(self, db) -> None: return_value=provider, ), ): - _, count = build_and_upload_errors( + build_and_upload_errors( session=db, execution=execution, assessment=assessment, bag=_bag(stage_batches={ApiStage.ASSESSMENT.value: job.id}), failure_message=None, ) - - assert count == 1 row = uploads.rows[0] assert row["type"] == "provider_error_file_unavailable" assert row["provider_error_file_id"] == "file-err-2" @@ -311,7 +300,7 @@ def test_batch_without_an_error_file_contributes_nothing(self, db) -> None: uploads = _Uploads() with _storage_patch(), _upload_patch(uploads): - _, count = build_and_upload_errors( + build_and_upload_errors( session=db, execution=execution, assessment=assessment, @@ -319,8 +308,6 @@ def test_batch_without_an_error_file_contributes_nothing(self, db) -> None: failure_message=None, ) - assert count == 0 - def test_storage_outage_yields_no_url(self, db) -> None: auth = get_user_test_auth_context(db) assessment, execution = _seed(db, auth) @@ -329,7 +316,7 @@ def test_storage_outage_yields_no_url(self, db) -> None: "app.services.assessment.api.result_files.get_cloud_storage", side_effect=RuntimeError("s3 unreachable"), ): - url, count = build_and_upload_errors( + url = build_and_upload_errors( session=db, execution=execution, assessment=assessment, @@ -338,7 +325,6 @@ def test_storage_outage_yields_no_url(self, db) -> None: ) assert url is None - assert count == 1 class TestFinalizeResultFiles: @@ -361,8 +347,7 @@ def test_pre_provider_failure_records_only_a_synthetic_execution_error( db.refresh(assessment) assert set(assessment.result_files) == {"errors"} assert assessment.result_files["errors"] == { - "url": "s3://bucket/errors.jsonl", - "count": 1, + "object_store_url": "s3://bucket/errors.jsonl", } assert uploads.rows == [ { @@ -382,7 +367,6 @@ def test_completed_run_carries_both_a_results_and_an_errors_record( assessment=assessment, stage=ApiStage.ASSESSMENT.value, url="s3://bucket/batch-1173/output.jsonl", - count=2, ) uploads = _Uploads() @@ -400,8 +384,8 @@ def test_completed_run_carries_both_a_results_and_an_errors_record( db.refresh(assessment) assert assessment.result_files == { - "results": {"url": "s3://bucket/batch-1173/output.jsonl", "count": 2}, - "errors": {"url": "s3://bucket/errors.jsonl", "count": 0}, + "results": {"object_store_url": "s3://bucket/batch-1173/output.jsonl"}, + "errors": {"object_store_url": "s3://bucket/errors.jsonl"}, } def test_prefilter_and_assessment_dumps_coexist(self, db) -> None: @@ -434,7 +418,7 @@ def test_prefilter_and_assessment_dumps_coexist(self, db) -> None: "results", "errors", } - assert assessment.result_files["topic_relevance_results"]["count"] == 2 + assert "topic_relevance_results" in assessment.result_files def test_a_second_tick_does_not_duplicate_or_lose_records(self, db) -> None: auth = get_user_test_auth_context(db) @@ -496,15 +480,15 @@ def sign(url, expires_in): storage.get_signed_url.side_effect = sign return storage - def test_every_kind_is_signed_and_keeps_its_count(self, db) -> None: + def test_every_kind_is_signed(self, db) -> None: auth = get_user_test_auth_context(db) assessment, _ = _seed(db, auth) api.set_result_files( session=db, assessment=assessment, files={ - "results": {"url": "s3://bucket/out.jsonl", "count": 998}, - "errors": {"url": "s3://bucket/errors.jsonl", "count": 389}, + "results": {"object_store_url": "s3://bucket/out.jsonl"}, + "errors": {"object_store_url": "s3://bucket/errors.jsonl"}, }, ) @@ -512,10 +496,8 @@ def test_every_kind_is_signed_and_keeps_its_count(self, db) -> None: metadata = build_callback_metadata(session=db, assessment=assessment) assert metadata["result_files"]["results"] == { - "url": f"https://signed.example/s3://bucket/out.jsonl?exp={ONE_DAY_SECONDS}", - "count": 998, + "signed_url": f"https://signed.example/s3://bucket/out.jsonl?exp={ONE_DAY_SECONDS}", } - assert metadata["result_files"]["errors"]["count"] == 389 def test_expires_at_is_one_day_out(self, db) -> None: auth = get_user_test_auth_context(db) @@ -534,8 +516,8 @@ def test_a_failing_presign_drops_only_its_own_kind(self, db) -> None: session=db, assessment=assessment, files={ - "results": {"url": "s3://bucket/out.jsonl", "count": 998}, - "errors": {"url": "s3://bucket/errors.jsonl", "count": 389}, + "results": {"object_store_url": "s3://bucket/out.jsonl"}, + "errors": {"object_store_url": "s3://bucket/errors.jsonl"}, }, ) @@ -551,7 +533,7 @@ def test_storage_outage_still_returns_the_envelope_keys(self, db) -> None: api.set_result_files( session=db, assessment=assessment, - files={"results": {"url": "s3://bucket/out.jsonl", "count": 1}}, + files={"results": {"object_store_url": "s3://bucket/out.jsonl"}}, ) with patch( diff --git a/backend/app/tests/assessment/test_routes.py b/backend/app/tests/assessment/test_routes.py index a2fdf9a65..3d7f925cb 100644 --- a/backend/app/tests/assessment/test_routes.py +++ b/backend/app/tests/assessment/test_routes.py @@ -16,7 +16,7 @@ retry_assessment, ) from app.api.routes.assessment.datasets import ( - _dataset_to_response, + _submission_to_response, delete_dataset, get_dataset, list_datasets, @@ -47,13 +47,13 @@ def _auth_context() -> SimpleNamespace: ) -def _dataset() -> SimpleNamespace: +def _submission() -> SimpleNamespace: return SimpleNamespace( - id=7, + id=UUID(int=7), name="ds", description="d", - dataset_metadata={"total_items_count": 2, "file_extension": ".csv"}, - object_store_url="s3://x", + total_items=2, + object_store_url="s3://x/ds.csv", ) @@ -61,7 +61,7 @@ def _assessment() -> SimpleNamespace: return SimpleNamespace( id=10, experiment_name="exp", - dataset_id=7, + submission_id=UUID(int=7), status="processing", organization_id=1, project_id=1, @@ -111,9 +111,9 @@ def _row(execution_id: int = 22) -> AssessmentExportRow: class TestRouteHelpers: - def test_dataset_to_response(self) -> None: - resp = _dataset_to_response(_dataset(), signed_url="signed") - assert resp.dataset_id == 7 + def test_submission_to_response(self) -> None: + resp = _submission_to_response(_submission(), signed_url="signed") + assert resp.submission_id == UUID(int=7) assert resp.signed_url == "signed" @@ -123,8 +123,8 @@ def test_dataset_to_response(self) -> None: class TestDatasetRoutes: def test_list_datasets(self) -> None: with patch( - "app.api.routes.assessment.datasets.list_assessment_datasets", - return_value=[_dataset()], + "app.api.routes.assessment.datasets.list_submissions", + return_value=[_submission()], ): resp = list_datasets(session=MagicMock(), auth_context=_auth_context()) assert resp.success is True @@ -132,7 +132,7 @@ def test_list_datasets(self) -> None: def test_get_dataset_not_found(self) -> None: with patch( - "app.api.routes.assessment.datasets.get_assessment_dataset_by_id", + "app.api.routes.assessment.datasets.get_submission_by_id", side_effect=HTTPException( status_code=404, detail="Dataset 1 not found or not accessible", @@ -145,8 +145,8 @@ def test_get_dataset_with_signed_url(self) -> None: storage = MagicMock() storage.get_signed_url.return_value = "signed-url" with patch( - "app.api.routes.assessment.datasets.get_assessment_dataset_by_id", - return_value=_dataset(), + "app.api.routes.assessment.datasets.get_submission_by_id", + return_value=_submission(), ), patch( "app.api.routes.assessment.datasets.get_cloud_storage", return_value=storage ): @@ -162,10 +162,10 @@ def test_get_dataset_with_signed_url(self) -> None: def test_get_dataset_with_limit_rows_includes_preview(self) -> None: with patch( - "app.api.routes.assessment.datasets.get_assessment_dataset_by_id", - return_value=_dataset(), + "app.api.routes.assessment.datasets.get_submission_by_id", + return_value=_submission(), ), patch( - "app.api.routes.assessment.datasets.preview_assessment_dataset", + "app.api.routes.assessment.datasets.preview_submission", return_value=(["a", "b"], [["1", "2"], ["3", "4"]]), ) as preview_mock: resp = get_dataset( @@ -183,10 +183,10 @@ def test_get_dataset_with_limit_rows_includes_preview(self) -> None: def test_get_dataset_without_limit_rows_skips_preview(self) -> None: with patch( - "app.api.routes.assessment.datasets.get_assessment_dataset_by_id", - return_value=_dataset(), + "app.api.routes.assessment.datasets.get_submission_by_id", + return_value=_submission(), ), patch( - "app.api.routes.assessment.datasets.preview_assessment_dataset" + "app.api.routes.assessment.datasets.preview_submission" ) as preview_mock: resp = get_dataset(7, session=MagicMock(), auth_context=_auth_context()) preview_mock.assert_not_called() @@ -195,20 +195,20 @@ def test_get_dataset_without_limit_rows_skips_preview(self) -> None: def test_delete_dataset_success_and_error(self) -> None: with patch( - "app.api.routes.assessment.datasets.get_assessment_dataset_by_id", - return_value=_dataset(), + "app.api.routes.assessment.datasets.get_submission_by_id", + return_value=_submission(), ), patch( - "app.api.routes.assessment.datasets.delete_assessment_dataset", + "app.api.routes.assessment.datasets.delete_submission", return_value=None, ): resp = delete_dataset(7, session=MagicMock(), auth_context=_auth_context()) assert resp.success is True with patch( - "app.api.routes.assessment.datasets.get_assessment_dataset_by_id", - return_value=_dataset(), + "app.api.routes.assessment.datasets.get_submission_by_id", + return_value=_submission(), ), patch( - "app.api.routes.assessment.datasets.delete_assessment_dataset", + "app.api.routes.assessment.datasets.delete_submission", return_value="cannot delete", ): with pytest.raises(HTTPException, match="cannot delete"): @@ -222,7 +222,7 @@ class TestRunRoutes: def test_create_assessment_runs(self) -> None: request = AssessmentRunCreate( experiment_name="exp", - dataset_id=7, + submission_id=UUID(int=7), input_binding=InputBinding(prompt="p", text_columns=[], attachments=[]), configs=[ AssessmentConfigRef( @@ -233,8 +233,8 @@ def test_create_assessment_runs(self) -> None: result = SimpleNamespace( assessment_id=10, experiment_name="exp", - dataset_id=7, - dataset_name="ds", + submission_id=UUID(int=7), + submission_name="ds", num_configs=1, runs=[], ) @@ -250,8 +250,8 @@ def test_retry_endpoints(self) -> None: result = SimpleNamespace( assessment_id=10, experiment_name="exp", - dataset_id=7, - dataset_name="ds", + submission_id=UUID(int=7), + submission_name="ds", num_configs=1, runs=[], ) diff --git a/backend/app/tests/assessment/test_service.py b/backend/app/tests/assessment/test_service.py index 7460676e9..6b0508cd6 100644 --- a/backend/app/tests/assessment/test_service.py +++ b/backend/app/tests/assessment/test_service.py @@ -33,7 +33,7 @@ def _make_request(provider_config_id: UUID) -> AssessmentRunCreate: return AssessmentRunCreate( experiment_name="exp-1", - dataset_id=7, + submission_id=UUID(int=7), input_binding=InputBinding( prompt="Answer: {question}", text_columns=["question"], @@ -80,7 +80,7 @@ def test_dataset_not_found(self) -> None: session = MagicMock() request = _make_request(UUID("00000000-0000-0000-0000-000000000001")) with patch( - "app.services.assessment.service.get_assessment_dataset_by_id", + "app.services.assessment.service.get_submission_by_id", side_effect=HTTPException( status_code=404, detail="Dataset 7 not found or not accessible", @@ -99,7 +99,7 @@ def test_config_resolution_failure(self) -> None: request = _make_request(UUID("00000000-0000-0000-0000-000000000001")) with ( patch( - "app.services.assessment.service.get_assessment_dataset_by_id", + "app.services.assessment.service.get_submission_by_id", return_value=_make_dataset(), ), patch( @@ -123,7 +123,7 @@ def test_rejects_unsupported_provider(self) -> None: with ( patch( - "app.services.assessment.service.get_assessment_dataset_by_id", + "app.services.assessment.service.get_submission_by_id", return_value=_make_dataset(), ), patch( @@ -159,7 +159,7 @@ def test_google_provider_is_supported(self) -> None: with ( patch( - "app.services.assessment.service.get_assessment_dataset_by_id", + "app.services.assessment.service.get_submission_by_id", return_value=dataset, ), patch( @@ -215,7 +215,7 @@ def test_supported_batch_providers_are_accepted(self, provider: str) -> None: with ( patch( - "app.services.assessment.service.get_assessment_dataset_by_id", + "app.services.assessment.service.get_submission_by_id", return_value=dataset, ), patch( @@ -259,7 +259,7 @@ def test_anthropic_provider_is_supported(self) -> None: with ( patch( - "app.services.assessment.service.get_assessment_dataset_by_id", + "app.services.assessment.service.get_submission_by_id", return_value=dataset, ), patch( @@ -303,7 +303,7 @@ def test_defaults_missing_provider_to_openai(self) -> None: with ( patch( - "app.services.assessment.service.get_assessment_dataset_by_id", + "app.services.assessment.service.get_submission_by_id", return_value=dataset, ), patch( @@ -350,7 +350,7 @@ def test_rejects_default_tagged_config(self) -> None: with ( patch( - "app.services.assessment.service.get_assessment_dataset_by_id", + "app.services.assessment.service.get_submission_by_id", return_value=_make_dataset(), ), patch("app.services.assessment.service.ConfigCrud", return_value=crud), @@ -388,7 +388,7 @@ def test_dispatches_one_celery_task_per_config(self) -> None: with ( patch( - "app.services.assessment.service.get_assessment_dataset_by_id", + "app.services.assessment.service.get_submission_by_id", return_value=dataset, ), patch( @@ -428,13 +428,19 @@ def test_build_retry_request_errors_and_success(self) -> None: with pytest.raises(HTTPException, match="No assessment runs"): _build_retry_request( - experiment_name="exp", dataset_id=1, input_binding=binding, runs=[] + experiment_name="exp", + submission_id=UUID(int=1), + input_binding=binding, + runs=[], ) run = MagicMock() with pytest.raises(HTTPException, match="missing for retry"): _build_retry_request( - experiment_name="exp", dataset_id=1, input_binding=None, runs=[run] + experiment_name="exp", + submission_id=UUID(int=1), + input_binding=None, + runs=[run], ) run2 = MagicMock() @@ -443,7 +449,10 @@ def test_build_retry_request_errors_and_success(self) -> None: run2.config_version = None with pytest.raises(HTTPException, match="Config reference is missing"): _build_retry_request( - experiment_name="exp", dataset_id=1, input_binding=binding, runs=[run2] + experiment_name="exp", + submission_id=UUID(int=1), + input_binding=binding, + runs=[run2], ) run3 = MagicMock() @@ -451,7 +460,10 @@ def test_build_retry_request_errors_and_success(self) -> None: run3.config_id = CONFIG_ID run3.config_version = 1 req = _build_retry_request( - experiment_name="exp", dataset_id=1, input_binding=binding, runs=[run3] + experiment_name="exp", + submission_id=UUID(int=1), + input_binding=binding, + runs=[run3], ) assert req.experiment_name == "exp" assert req.input_binding.prompt == "p" @@ -462,7 +474,7 @@ def test_retry_assessment_wrappers(self) -> None: assessment = MagicMock() assessment.id = ASSESSMENT_ID assessment.experiment_name = "exp" - assessment.dataset_id = 7 + assessment.submission_id = UUID(int=7) assessment.input = {"prompt": "p", "text_columns": [], "attachments": []} run = MagicMock() run.assessment_id = ASSESSMENT_ID @@ -473,8 +485,8 @@ def test_retry_assessment_wrappers(self) -> None: result = SimpleNamespace( assessment_id=1, experiment_name="exp", - dataset_id=7, - dataset_name="ds", + submission_id=UUID(int=7), + submission_name="ds", num_configs=1, runs=[], ) @@ -518,7 +530,7 @@ def _failed_run(self, stage: str) -> MagicMock: }, } run.assessment = SimpleNamespace( - id=ASSESSMENT_ID, experiment_name="exp", dataset_id=7 + id=ASSESSMENT_ID, experiment_name="exp", submission_id=UUID(int=7) ) return run @@ -541,7 +553,7 @@ def test_resumes_in_place_from_failed_stage(self) -> None: with ( patch( - "app.services.assessment.service.get_assessment_dataset_by_id", + "app.services.assessment.service.get_submission_by_id", return_value=_make_dataset(), ), patch("app.services.assessment.service.recompute_assessment_status"), diff --git a/backend/app/tests/assessment/test_dataset.py b/backend/app/tests/assessment/test_submission.py similarity index 58% rename from backend/app/tests/assessment/test_dataset.py rename to backend/app/tests/assessment/test_submission.py index 2535c2acd..abe7b2f32 100644 --- a/backend/app/tests/assessment/test_dataset.py +++ b/backend/app/tests/assessment/test_submission.py @@ -1,4 +1,4 @@ -"""Tests for assessment/dataset.py upload and row counting behavior.""" +"""Tests for assessment/submission.py upload and row counting behavior.""" from unittest.mock import MagicMock, patch @@ -6,14 +6,14 @@ from fastapi import HTTPException from openpyxl.utils.exceptions import InvalidFileException -from app.services.assessment.dataset import ( +from app.services.assessment.submission import ( _count_csv_rows, _count_excel_rows, _count_rows, _preview_csv, _preview_excel, - preview_dataset, - upload_dataset, + preview_submission, + upload_submission, ) @@ -39,26 +39,32 @@ def test_count_csv_rows(self) -> None: assert _count_csv_rows(b"a,b\n1,2\n\n3,4\n") == 2 def test_count_rows_csv_and_xlsx(self) -> None: - with patch("app.services.assessment.dataset._count_excel_rows", return_value=5): + with patch( + "app.services.assessment.submission._count_excel_rows", return_value=5 + ): assert _count_rows(b"x", ".xlsx") == 5 assert _count_rows(b"a,b\n1,2\n", ".csv") == 1 -class TestUploadDataset: +class TestUploadSubmission: def test_invalid_xlsx_returns_422(self) -> None: session = MagicMock() with patch( - "app.services.assessment.dataset.sanitize_dataset_name", return_value="ds-1" + "app.services.assessment.submission.sanitize_dataset_name", + return_value="ds-1", + ), patch( + "app.services.assessment.submission.get_submission_by_name", + return_value=None, ), patch( - "app.services.assessment.dataset._count_rows", + "app.services.assessment.submission._count_rows", side_effect=InvalidFileException("bad xlsx"), ): with pytest.raises(HTTPException) as exc_info: - upload_dataset( + upload_submission( session=session, file_content=b"invalid-xlsx", file_ext=".xlsx", - dataset_name="ds-1", + submission_name="ds-1", description=None, organization_id=1, project_id=1, @@ -69,17 +75,21 @@ def test_invalid_xlsx_returns_422(self) -> None: def test_count_rows_value_error_returns_422(self) -> None: session = MagicMock() with patch( - "app.services.assessment.dataset.sanitize_dataset_name", return_value="ds-1" + "app.services.assessment.submission.sanitize_dataset_name", + return_value="ds-1", ), patch( - "app.services.assessment.dataset._count_rows", + "app.services.assessment.submission.get_submission_by_name", + return_value=None, + ), patch( + "app.services.assessment.submission._count_rows", side_effect=ValueError("Legacy Excel format (.xls) is not supported."), ): with pytest.raises(HTTPException) as exc_info: - upload_dataset( + upload_submission( session=session, file_content=b"bad", file_ext=".xls", - dataset_name="ds-1", + submission_name="ds-1", description=None, organization_id=1, project_id=1, @@ -90,49 +100,62 @@ def test_count_rows_value_error_returns_422(self) -> None: def test_count_rows_unexpected_error_returns_generic_422(self) -> None: session = MagicMock() with patch( - "app.services.assessment.dataset.sanitize_dataset_name", return_value="ds-1" + "app.services.assessment.submission.sanitize_dataset_name", + return_value="ds-1", ), patch( - "app.services.assessment.dataset._count_rows", + "app.services.assessment.submission.get_submission_by_name", + return_value=None, + ), patch( + "app.services.assessment.submission._count_rows", side_effect=RuntimeError("unexpected"), ): with pytest.raises(HTTPException) as exc_info: - upload_dataset( + upload_submission( session=session, file_content=b"bad", file_ext=".xlsx", - dataset_name="ds-1", + submission_name="ds-1", description=None, organization_id=1, project_id=1, ) assert exc_info.value.status_code == 422 - assert "Unable to parse dataset file" in exc_info.value.detail + assert "Unable to parse the file" in exc_info.value.detail - def test_upload_dataset_success(self) -> None: + def test_upload_submission_success(self) -> None: session = MagicMock() created = MagicMock() created.id = 9 with patch( - "app.services.assessment.dataset.sanitize_dataset_name", return_value="ds-1" - ), patch("app.services.assessment.dataset._count_rows", return_value=2), patch( - "app.services.assessment.dataset._upload_file_to_object_store", + "app.services.assessment.submission.sanitize_dataset_name", + return_value="ds-1", + ), patch( + "app.services.assessment.submission.get_submission_by_name", + return_value=None, + ), patch( + "app.services.assessment.submission.get_submission_by_name", + return_value=None, + ), patch( + "app.services.assessment.submission._count_rows", return_value=2 + ), patch( + "app.services.assessment.submission._upload_file_to_object_store", return_value="s3://datasets/file.csv", ), patch( - "app.services.assessment.dataset.create_assessment_dataset", + "app.services.assessment.submission.create_submission", return_value=created, ) as create_ds: - result = upload_dataset( + result = upload_submission( session=session, file_content=b"a,b\n1,2\n", file_ext=".csv", - dataset_name="ds-1", + submission_name="ds-1", description="desc", organization_id=1, project_id=1, ) assert result.id == 9 create_ds.assert_called_once() - assert create_ds.call_args.kwargs["dataset_metadata"]["total_items_count"] == 2 + assert create_ds.call_args.kwargs["total_items"] == 2 def test_preview_csv_returns_headers_and_rows(self) -> None: headers, rows = _preview_csv(b"a,b\n1,2\n\n3,4\n5,6\n", limit=2) @@ -174,129 +197,149 @@ def test_preview_excel_empty_workbook(self) -> None: assert headers == [""] or headers == [] assert rows == [] - def test_preview_dataset_missing_url_returns_404(self) -> None: + def test_preview_submission_missing_url_returns_404(self) -> None: ds = MagicMock() ds.object_store_url = None with pytest.raises(HTTPException) as exc_info: - preview_dataset(session=MagicMock(), dataset=ds, project_id=1, limit=10) + preview_submission( + session=MagicMock(), submission=ds, project_id=1, limit=10 + ) assert exc_info.value.status_code == 404 - def test_preview_dataset_missing_extension_returns_422(self) -> None: + def test_preview_submission_missing_extension_returns_422(self) -> None: ds = MagicMock() ds.object_store_url = "s3://x" - ds.dataset_metadata = {} + ds.object_store_url = "s3://bucket/key" with pytest.raises(HTTPException) as exc_info: - preview_dataset(session=MagicMock(), dataset=ds, project_id=1, limit=10) + preview_submission( + session=MagicMock(), submission=ds, project_id=1, limit=10 + ) assert exc_info.value.status_code == 422 assert "Unsupported or missing" in exc_info.value.detail - def test_preview_dataset_unknown_extension_returns_422(self) -> None: + def test_preview_submission_unknown_extension_returns_422(self) -> None: ds = MagicMock() ds.object_store_url = "s3://x" - ds.dataset_metadata = {"file_extension": ".json"} + ds.object_store_url = "s3://bucket/key.json" with pytest.raises(HTTPException) as exc_info: - preview_dataset(session=MagicMock(), dataset=ds, project_id=1, limit=10) + preview_submission( + session=MagicMock(), submission=ds, project_id=1, limit=10 + ) assert exc_info.value.status_code == 422 - def test_preview_dataset_normalizes_extension_case(self) -> None: + def test_preview_submission_normalizes_extension_case(self) -> None: ds = MagicMock() ds.object_store_url = "s3://x" - ds.dataset_metadata = {"file_extension": " .CSV "} + ds.object_store_url = "s3://bucket/key.CSV" storage = MagicMock() storage.get.return_value = b"a,b\n1,2\n" with patch( - "app.services.assessment.dataset.get_cloud_storage", return_value=storage + "app.services.assessment.submission.get_cloud_storage", return_value=storage ): - headers, rows = preview_dataset( - session=MagicMock(), dataset=ds, project_id=1, limit=10 + headers, rows = preview_submission( + session=MagicMock(), submission=ds, project_id=1, limit=10 ) assert headers == ["a", "b"] assert rows == [["1", "2"]] - def test_preview_dataset_legacy_xls_returns_422(self) -> None: + def test_preview_submission_legacy_xls_returns_422(self) -> None: ds = MagicMock() ds.object_store_url = "s3://x" - ds.dataset_metadata = {"file_extension": ".xls"} + ds.object_store_url = "s3://bucket/key.xls" with pytest.raises(HTTPException) as exc_info: - preview_dataset(session=MagicMock(), dataset=ds, project_id=1, limit=10) + preview_submission( + session=MagicMock(), submission=ds, project_id=1, limit=10 + ) assert exc_info.value.status_code == 422 - def test_preview_dataset_storage_failure_returns_502(self) -> None: + def test_preview_submission_storage_failure_returns_502(self) -> None: ds = MagicMock() ds.object_store_url = "s3://x" - ds.dataset_metadata = {"file_extension": ".csv"} + ds.object_store_url = "s3://bucket/key.csv" storage = MagicMock() storage.get.side_effect = RuntimeError("boom") with patch( - "app.services.assessment.dataset.get_cloud_storage", return_value=storage + "app.services.assessment.submission.get_cloud_storage", return_value=storage ): with pytest.raises(HTTPException) as exc_info: - preview_dataset(session=MagicMock(), dataset=ds, project_id=1, limit=10) + preview_submission( + session=MagicMock(), submission=ds, project_id=1, limit=10 + ) assert exc_info.value.status_code == 502 - def test_preview_dataset_invalid_xlsx_returns_422(self) -> None: + def test_preview_submission_invalid_xlsx_returns_422(self) -> None: ds = MagicMock() ds.object_store_url = "s3://x" - ds.dataset_metadata = {"file_extension": ".xlsx"} + ds.object_store_url = "s3://bucket/key.xlsx" storage = MagicMock() storage.get.return_value = b"not-a-real-xlsx" with patch( - "app.services.assessment.dataset.get_cloud_storage", return_value=storage + "app.services.assessment.submission.get_cloud_storage", return_value=storage ), patch( - "app.services.assessment.dataset._preview_excel", + "app.services.assessment.submission._preview_excel", side_effect=InvalidFileException("bad"), ): with pytest.raises(HTTPException) as exc_info: - preview_dataset(session=MagicMock(), dataset=ds, project_id=1, limit=10) + preview_submission( + session=MagicMock(), submission=ds, project_id=1, limit=10 + ) assert exc_info.value.status_code == 422 assert "Invalid XLSX" in exc_info.value.detail - def test_preview_dataset_parse_error_returns_422(self) -> None: + def test_preview_submission_parse_error_returns_422(self) -> None: ds = MagicMock() ds.object_store_url = "s3://x" - ds.dataset_metadata = {"file_extension": ".csv"} + ds.object_store_url = "s3://bucket/key.csv" storage = MagicMock() storage.get.return_value = b"a,b\n1,2\n" with patch( - "app.services.assessment.dataset.get_cloud_storage", return_value=storage + "app.services.assessment.submission.get_cloud_storage", return_value=storage ), patch( - "app.services.assessment.dataset._preview_csv", + "app.services.assessment.submission._preview_csv", side_effect=RuntimeError("boom"), ): with pytest.raises(HTTPException) as exc_info: - preview_dataset(session=MagicMock(), dataset=ds, project_id=1, limit=10) + preview_submission( + session=MagicMock(), submission=ds, project_id=1, limit=10 + ) assert exc_info.value.status_code == 422 assert "Unable to parse" in exc_info.value.detail - def test_preview_dataset_csv_success(self) -> None: + def test_preview_submission_csv_success(self) -> None: ds = MagicMock() ds.object_store_url = "s3://x" - ds.dataset_metadata = {"file_extension": ".csv"} + ds.object_store_url = "s3://bucket/key.csv" storage = MagicMock() storage.get.return_value = b"a,b\n1,2\n3,4\n" with patch( - "app.services.assessment.dataset.get_cloud_storage", return_value=storage + "app.services.assessment.submission.get_cloud_storage", return_value=storage ): - headers, rows = preview_dataset( - session=MagicMock(), dataset=ds, project_id=1, limit=10 + headers, rows = preview_submission( + session=MagicMock(), submission=ds, project_id=1, limit=10 ) assert headers == ["a", "b"] assert rows == [["1", "2"], ["3", "4"]] - def test_upload_dataset_object_store_failure_returns_500(self) -> None: + def test_upload_submission_object_store_failure_returns_500(self) -> None: session = MagicMock() with patch( - "app.services.assessment.dataset.sanitize_dataset_name", return_value="ds-1" - ), patch("app.services.assessment.dataset._count_rows", return_value=1), patch( - "app.services.assessment.dataset._upload_file_to_object_store", + "app.services.assessment.submission.sanitize_dataset_name", + return_value="ds-1", + ), patch( + "app.services.assessment.submission.get_submission_by_name", + return_value=None, + ), patch( + "app.services.assessment.submission._count_rows", return_value=1 + ), patch( + "app.services.assessment.submission._upload_file_to_object_store", return_value=None, ): with pytest.raises(HTTPException) as exc_info: - upload_dataset( + upload_submission( session=session, file_content=b"a,b\n1,2\n", file_ext=".csv", - dataset_name="ds-1", + submission_name="ds-1", description=None, organization_id=1, project_id=1, From 55820af919ff042d3b0f44689324e46fcf811e42 Mon Sep 17 00:00:00 2001 From: Prashant Vasudevan <71649489+vprashrex@users.noreply.github.com> Date: Thu, 10 Sep 2026 11:47:03 +0530 Subject: [PATCH 3/4] test(assessment): cover the submission crud and storage round trip Patch coverage was below target on the new submission path: the crud, the object-store round trip and the submission_doc_id branch had none. --- .../tests/assessment/test_api_submission.py | 100 +++++++++++++ .../tests/assessment/test_submission_crud.py | 137 ++++++++++++++++++ .../tests/assessment/test_submission_store.py | 84 +++++++++++ 3 files changed, 321 insertions(+) create mode 100644 backend/app/tests/assessment/test_submission_crud.py create mode 100644 backend/app/tests/assessment/test_submission_store.py diff --git a/backend/app/tests/assessment/test_api_submission.py b/backend/app/tests/assessment/test_api_submission.py index 554617009..c5f73fa66 100644 --- a/backend/app/tests/assessment/test_api_submission.py +++ b/backend/app/tests/assessment/test_api_submission.py @@ -10,6 +10,7 @@ from app.core.config import settings from app.crud.assessment import api +from app.crud.assessment.submission import create_submission from app.models.assessment import ( Assessment, AssessmentCreate, @@ -278,6 +279,105 @@ def test_dispatch_failure_marks_failed_and_503(self, db) -> None: assert latest.status == AssessmentStatus.FAILED +class TestSubmissionDocId: + """The `submission_doc_id` branch: rows come from a stored submission file.""" + + @pytest.fixture(autouse=True) + def _bypass_callback_check(self): + with ( + patch("app.services.assessment.api.submission.validate_callback_url"), + patch( + "app.services.assessment.api.submission.upload_submission_rows", + return_value="s3://bucket/submission.jsonl", + ), + ): + yield + + def _request(self, config, submission_doc_id): + return AssessmentCreate.model_validate( + { + "config": {"id": str(config.id), "version": 1}, + "input": {"submission_doc_id": str(submission_doc_id)}, + "callback_url": "https://hook.example/cb", + } + ) + + def _submit(self, db, auth, config, submission_doc_id): + return submission.submit( + session=db, + request=self._request(config, submission_doc_id), + organization_id=auth.organization_id, + project_id=auth.project_id, + ) + + def test_rows_are_read_from_the_submission(self, db) -> None: + auth = get_user_test_auth_context(db) + config = _assessment_config(db, auth.project_id) + stored = create_submission( + session=db, + name="rows", + object_store_url="s3://bucket/rows.csv", + total_items=2, + organization_id=auth.organization_id, + project_id=auth.project_id, + ) + + with patch( + "app.services.assessment.api.submission.load_submission_file_rows", + return_value=[{"a": "one"}, {"a": "two"}], + ), patch("app.celery.tasks.job_execution.run_assessment_api_batch"): + response = self._submit(db, auth, config, stored.id) + + assert response.status == AssessmentStatus.PROCESSING + + def test_unknown_submission_is_404(self, db) -> None: + auth = get_user_test_auth_context(db) + config = _assessment_config(db, auth.project_id) + with pytest.raises(HTTPException) as exc: + self._submit(db, auth, config, uuid4()) + assert exc.value.status_code == 404 + + def test_unreadable_submission_is_502(self, db) -> None: + auth = get_user_test_auth_context(db) + config = _assessment_config(db, auth.project_id) + stored = create_submission( + session=db, + name="unreadable", + object_store_url="s3://bucket/rows.csv", + total_items=1, + organization_id=auth.organization_id, + project_id=auth.project_id, + ) + + with patch( + "app.services.assessment.api.submission.load_submission_file_rows", + side_effect=RuntimeError("s3 down"), + ): + with pytest.raises(HTTPException) as exc: + self._submit(db, auth, config, stored.id) + assert exc.value.status_code == 502 + + def test_empty_submission_is_422(self, db) -> None: + auth = get_user_test_auth_context(db) + config = _assessment_config(db, auth.project_id) + stored = create_submission( + session=db, + name="empty", + object_store_url="s3://bucket/rows.csv", + total_items=0, + organization_id=auth.organization_id, + project_id=auth.project_id, + ) + + with patch( + "app.services.assessment.api.submission.load_submission_file_rows", + return_value=[], + ): + with pytest.raises(HTTPException) as exc: + self._submit(db, auth, config, stored.id) + assert exc.value.status_code == 422 + + class TestCallbackUrlValidation: """BUG 3 regression: submission.submit validates callback_url up front (HTTPS + SSRF/private-IP guard) and maps failure to 422, instead of only at delivery time.""" diff --git a/backend/app/tests/assessment/test_submission_crud.py b/backend/app/tests/assessment/test_submission_crud.py new file mode 100644 index 000000000..9e0162eda --- /dev/null +++ b/backend/app/tests/assessment/test_submission_crud.py @@ -0,0 +1,137 @@ +"""Tests for assessment submission CRUD (app/crud/assessment/submission.py).""" + +from uuid import uuid4 + +import pytest +from fastapi import HTTPException + +from app.crud.assessment import api +from app.crud.assessment.submission import ( + create_submission, + delete_submission, + get_submission_by_id, + get_submission_by_name, + list_submissions, +) +from app.models.assessment import AssessmentMethod +from app.tests.utils.auth import get_user_test_auth_context +from app.tests.utils.utils import random_lower_string + + +def _create(db, auth, **kwargs): + return create_submission( + session=db, + name=kwargs.pop("name", random_lower_string()), + object_store_url="s3://bucket/sub.csv", + total_items=kwargs.pop("total_items", 3), + organization_id=auth.organization_id, + project_id=auth.project_id, + **kwargs, + ) + + +class TestCreate: + def test_persists_the_row(self, db) -> None: + auth = get_user_test_auth_context(db) + submission = _create(db, auth, description="desc") + + assert submission.id is not None + assert submission.total_items == 3 + assert submission.description == "desc" + + def test_duplicate_name_is_409(self, db) -> None: + auth = get_user_test_auth_context(db) + name = random_lower_string() + _create(db, auth, name=name) + + with pytest.raises(HTTPException) as exc: + _create(db, auth, name=name) + assert exc.value.status_code == 409 + + +class TestReads: + def test_get_by_id_scoped_to_project(self, db) -> None: + auth = get_user_test_auth_context(db) + submission = _create(db, auth) + + found = get_submission_by_id( + session=db, + submission_id=submission.id, + organization_id=auth.organization_id, + project_id=auth.project_id, + ) + assert found.id == submission.id + + with pytest.raises(HTTPException) as exc: + get_submission_by_id( + session=db, + submission_id=submission.id, + organization_id=auth.organization_id, + project_id=auth.project_id + 1, + ) + assert exc.value.status_code == 404 + + def test_get_by_id_unknown_is_404(self, db) -> None: + auth = get_user_test_auth_context(db) + with pytest.raises(HTTPException) as exc: + get_submission_by_id( + session=db, + submission_id=uuid4(), + organization_id=auth.organization_id, + project_id=auth.project_id, + ) + assert exc.value.status_code == 404 + + def test_get_by_name_returns_none_when_absent(self, db) -> None: + auth = get_user_test_auth_context(db) + assert ( + get_submission_by_name( + session=db, + name=random_lower_string(), + organization_id=auth.organization_id, + project_id=auth.project_id, + ) + is None + ) + + def test_list_returns_the_project_rows(self, db) -> None: + auth = get_user_test_auth_context(db) + created = _create(db, auth) + + rows = list_submissions( + session=db, + organization_id=auth.organization_id, + project_id=auth.project_id, + ) + assert created.id in {row.id for row in rows} + + +class TestDelete: + def test_deletes_an_unreferenced_submission(self, db) -> None: + auth = get_user_test_auth_context(db) + submission = _create(db, auth) + + assert delete_submission(session=db, submission=submission) is None + with pytest.raises(HTTPException): + get_submission_by_id( + session=db, + submission_id=submission.id, + organization_id=auth.organization_id, + project_id=auth.project_id, + ) + + def test_refuses_while_an_assessment_references_it(self, db) -> None: + auth = get_user_test_auth_context(db) + submission = _create(db, auth) + api.create_assessment( + session=db, + method=AssessmentMethod.BATCH, + input=None, + submission_id=submission.id, + organization_id=auth.organization_id, + project_id=auth.project_id, + ) + + error = delete_submission(session=db, submission=submission) + assert error is not None + assert "being used by" in error diff --git a/backend/app/tests/assessment/test_submission_store.py b/backend/app/tests/assessment/test_submission_store.py new file mode 100644 index 000000000..ebe2f39a0 --- /dev/null +++ b/backend/app/tests/assessment/test_submission_store.py @@ -0,0 +1,84 @@ +"""Tests for the submission-rows round trip (api/submission_store.py).""" + +import io +from unittest.mock import MagicMock, patch +from uuid import uuid4 + +import pytest + +from app.models.assessment import Assessment, AssessmentMethod, BatchInput +from app.services.assessment.api.submission_store import ( + SubmissionUnavailableError, + load_submission_rows, + upload_submission_rows, +) + +_STORAGE = "app.services.assessment.api.submission_store.get_cloud_storage" +_UPLOAD = "app.services.assessment.api.submission_store.upload_jsonl_to_object_store" + + +def _assessment(url: str | None) -> Assessment: + return Assessment( + id=uuid4(), + method=AssessmentMethod.BATCH, + submission_input=url, + organization_id=1, + project_id=1, + ) + + +class TestUpload: + def test_writes_rows_under_the_assessment_prefix(self) -> None: + assessment_id = uuid4() + with patch(_STORAGE), patch(_UPLOAD, return_value="s3://b/sub.jsonl") as upload: + url = upload_submission_rows( + session=MagicMock(), + assessment_id=assessment_id, + project_id=1, + batch_input=BatchInput(data=[{"a": "1"}]), + ) + + assert url == "s3://b/sub.jsonl" + assert upload.call_args.kwargs["filename"] == "submission.jsonl" + assert ( + upload.call_args.kwargs["subdirectory"] == f"assessment/{assessment_id}" + ) + + def test_returns_none_when_the_upload_fails(self) -> None: + with patch(_STORAGE), patch(_UPLOAD, return_value=None): + assert ( + upload_submission_rows( + session=MagicMock(), + assessment_id=uuid4(), + project_id=1, + batch_input=BatchInput(data=[{"a": "1"}]), + ) + is None + ) + + +class TestLoad: + def test_parses_the_stored_jsonl(self) -> None: + storage = MagicMock() + storage.stream.return_value = io.BytesIO( + b'{"a": "1"}\n\n{"a": "2"}\n' + ) + with patch(_STORAGE, return_value=storage): + result = load_submission_rows( + session=MagicMock(), assessment=_assessment("s3://b/sub.jsonl") + ) + + assert result.data == [{"a": "1"}, {"a": "2"}] + + def test_missing_url_is_a_value_error(self) -> None: + with pytest.raises(ValueError, match="No submission_input"): + load_submission_rows(session=MagicMock(), assessment=_assessment(None)) + + def test_storage_failure_is_retryable(self) -> None: + storage = MagicMock() + storage.stream.side_effect = RuntimeError("s3 down") + with patch(_STORAGE, return_value=storage): + with pytest.raises(SubmissionUnavailableError): + load_submission_rows( + session=MagicMock(), assessment=_assessment("s3://b/sub.jsonl") + ) From a2a5bb15b445810d50058d8d86a1e742d3159371 Mon Sep 17 00:00:00 2001 From: Prashant Vasudevan <71649489+vprashrex@users.noreply.github.com> Date: Thu, 10 Sep 2026 12:02:25 +0530 Subject: [PATCH 4/4] test(assessment): cover the submission crud and storage round trip Patch coverage was below target on the new submission path: the crud, the object-store round trip and the submission_doc_id branch had none. --- backend/app/tests/assessment/test_submission_store.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/backend/app/tests/assessment/test_submission_store.py b/backend/app/tests/assessment/test_submission_store.py index ebe2f39a0..a76c502a3 100644 --- a/backend/app/tests/assessment/test_submission_store.py +++ b/backend/app/tests/assessment/test_submission_store.py @@ -40,9 +40,7 @@ def test_writes_rows_under_the_assessment_prefix(self) -> None: assert url == "s3://b/sub.jsonl" assert upload.call_args.kwargs["filename"] == "submission.jsonl" - assert ( - upload.call_args.kwargs["subdirectory"] == f"assessment/{assessment_id}" - ) + assert upload.call_args.kwargs["subdirectory"] == f"assessment/{assessment_id}" def test_returns_none_when_the_upload_fails(self) -> None: with patch(_STORAGE), patch(_UPLOAD, return_value=None): @@ -60,9 +58,7 @@ def test_returns_none_when_the_upload_fails(self) -> None: class TestLoad: def test_parses_the_stored_jsonl(self) -> None: storage = MagicMock() - storage.stream.return_value = io.BytesIO( - b'{"a": "1"}\n\n{"a": "2"}\n' - ) + storage.stream.return_value = io.BytesIO(b'{"a": "1"}\n\n{"a": "2"}\n') with patch(_STORAGE, return_value=storage): result = load_submission_rows( session=MagicMock(), assessment=_assessment("s3://b/sub.jsonl")