feat(assessment): Callback Metadata typed result files

backend/app/models/assessment/__init__.py

@@ -38,6 +38,7 @@
     AssessmentOutput,
     AssessmentResult,
     AssessmentResultData,
+    AssessmentResultFiles,
     AssessmentResultRow,
     AssessmentSubmitResponse,
     AssessmentSummary,
@@ -99,6 +100,7 @@
     "AssessmentCreate",
     "AssessmentSubmitResponse",
     "AssessmentCallback",
+    "AssessmentResultFiles",
     "PreFilterVerdict",
     "PreFilter",
     "AssessmentOutput",

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, model_validator
+from pydantic import BaseModel, ConfigDict, Field, HttpUrl, JsonValue, model_validator
 from sqlmodel import SQLModel
 
 from app.models.assessment.assessment import (
@@ -140,7 +140,7 @@ class AssessmentCreate(BaseModel):
             "GET /assessments/{assessment_id} instead"
         ),
     )
-    request_metadata: dict[str, Any] | None = Field(
+    request_metadata: dict[str, JsonValue] | None = Field(
         default=None,
         description="Passed through unchanged in the callback for correlation",
     )
@@ -260,4 +260,12 @@ class AssessmentCallback(BaseModel):
     assessment_id: UUID
     status: AssessmentStatus
     data: AssessmentResultData | None = None
-    request_metadata: dict[str, Any] | None = None
+    request_metadata: dict[str, JsonValue] | None = None
+
+
+class AssessmentResultFiles(BaseModel):
+    """URLs of the provider dumps for each stage, if any."""
+
+    topic_relevance: str | None = None
+    assessment: str | None = None
+    errors: str | None = None

backend/app/services/assessment/api/callbacks.py

@@ -6,8 +6,8 @@
 """
 
 import logging
-from typing import Any
 
+from pydantic import JsonValue
 from sqlmodel import Session
 
 from app.models.assessment import (
@@ -16,7 +16,7 @@
     AssessmentCallback,
     AssessmentStatus,
 )
-from app.services.assessment.api.result_files import build_callback_metadata
+from app.services.assessment.api.result_files import presign_result_files
 from app.utils import get_webhook_secret, send_callback
 
 logger = logging.getLogger(__name__)
@@ -28,7 +28,7 @@ def deliver(
     assessment: Assessment,
     result: AssessmentBatchResult,
     callback_url: str,
-    request_metadata: dict[str, Any] | None,
+    request_metadata: dict[str, JsonValue] | None,
     failure_message: str | None,
 ) -> bool:
     """POST the assessment result to ``callback_url`` (HMAC-signed). Returns whether it was sent.
@@ -47,31 +47,31 @@ def deliver(
     )
 
     try:
-        metadata = build_callback_metadata(session=session, assessment=assessment)
+        files = presign_result_files(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
+        files = None
 
     sent = send_callback(
         callback_url,
         {
             "success": assessment.status != AssessmentStatus.FAILED,
             "data": callback.model_dump(mode="json"),
             "error": failure_message,
-            "metadata": metadata,
+            "metadata": {"files": files.model_dump()} if files else None,
         },
         webhook_secret=webhook_secret,
     )
     logger.info(
-        "[deliver] Callback %s | assessment_id=%s | status=%s | result_files=%s",
+        "[deliver] Callback %s | assessment_id=%s | status=%s | files=%s",
         "sent" if sent else "failed",
         assessment.id,
         assessment.status,
-        sorted((metadata or {}).get("result_files", {})),
+        files,
     )
     return sent

backend/app/services/assessment/api/result_files.py

@@ -1,12 +1,11 @@
 """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.
+Records every provider dump on ``assessment.result_files`` as ``{stage: {object_store_url}}``,
+builds ``errors.jsonl`` only when there is something to report, and presigns both into the
+callback envelope. Nothing here raises: a missing dump degrades to a missing key.
 """
 
 import logging
-from datetime import timedelta
 from enum import StrEnum
 from typing import Any
 
@@ -15,17 +14,16 @@
 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,
+    AssessmentResultFiles,
     AssessmentRun,
     BatchRunState,
 )
 from app.models.batch_job import BatchJob
 from app.services.assessment.api.batch import (
-    ApiStage,
     _build_batch_provider,
     error_file_entries,
 )
@@ -36,7 +34,6 @@
 
 logger = logging.getLogger(__name__)
 
-RESULTS_FILE_KIND = "results"
 ERRORS_FILE_KIND = "errors"
 
 # 86400 is the storage layer's own ceiling, so the presigned urls live exactly one day.
@@ -52,13 +49,6 @@ class ErrorRecordEnum(StrEnum):
     PROVIDER_ERROR_FILE_UNAVAILABLE = "provider_error_file_unavailable"
 
 
-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,
@@ -81,7 +71,7 @@ def record_stage_dump(
     api.set_result_files(
         session=session,
         assessment=assessment,
-        files={stage_file_kind(stage): {"object_store_url": url}},
+        files={stage: {"object_store_url": url}},
     )
 
 
@@ -162,11 +152,8 @@ def build_and_upload_errors(
     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.
-    """
+    """Assemble and upload the run's ``errors.jsonl``; ``None`` when there is nothing
+    to report, so a clean run leaves no empty object behind."""
     rows: list[dict[str, Any]] = []
     if failure_message:
         rows.append(
@@ -193,6 +180,9 @@ def build_and_upload_errors(
                 )
             )
 
+    if not rows:
+        return None
+
     try:
         storage = get_cloud_storage(session=session, project_id=assessment.project_id)
     except Exception:
@@ -239,7 +229,7 @@ def finalize_result_files(
         for stage, url in (bag.get("stage_output_urls") or {}).items():
             if not url:
                 continue
-            files[stage_file_kind(stage)] = {"object_store_url": url}
+            files[stage] = {"object_store_url": url}
 
         errors_url = build_and_upload_errors(
             session=session,
@@ -263,27 +253,25 @@ def finalize_result_files(
         )
 
 
-def build_callback_metadata(
+def presign_result_files(
     *, session: Session, assessment: Assessment
-) -> dict[str, Any]:
+) -> AssessmentResultFiles:
     """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.
+    An unset field means no such dump; a presign failure leaves just that field null.
     """
-    expires_at = (now() + timedelta(seconds=SIGNED_URL_EXPIRY_SECONDS)).isoformat()
-    signed: dict[str, dict[str, Any]] = {}
+    files: dict[str, str] = {}
 
     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 | "
+            "[presign_result_files] Storage unavailable, sending empty files | "
             "assessment_id=%s",
             assessment.id,
             exc_info=True,
         )
-        return {"result_files": signed, "expires_at": expires_at}
+        return AssessmentResultFiles()
 
     for kind, record in assessment.result_files.items():
         entry: dict[str, Any] = record or {}
@@ -296,13 +284,13 @@ def build_callback_metadata(
             )
         except Exception:
             logger.error(
-                "[build_callback_metadata] Presign failed, dropping kind | "
+                "[presign_result_files] Presign failed, dropping kind | "
                 "assessment_id=%s | kind=%s",
                 assessment.id,
                 kind,
                 exc_info=True,
             )
             continue
-        signed[kind] = {"signed_url": signed_url}
+        files[kind] = signed_url
 
-    return {"result_files": signed, "expires_at": expires_at}
+    return AssessmentResultFiles.model_validate(files)

backend/app/tests/assessment/test_callbacks.py

@@ -0,0 +1,75 @@
+"""Tests for webhook delivery (app/services/assessment/api/callbacks.py)."""
+
+from unittest.mock import patch
+
+from app.models.assessment import AssessmentBatchResult
+from app.services.assessment.api.callbacks import deliver
+from app.tests.assessment.test_result_files import _seed
+from app.tests.utils.auth import get_user_test_auth_context
+
+
+def _result() -> AssessmentBatchResult:
+    return AssessmentBatchResult(total_items=1)
+
+
+class TestDeliver:
+    def test_sends_presigned_files_in_metadata(self, db) -> None:
+        auth = get_user_test_auth_context(db)
+        assessment, _ = _seed(db, auth)
+
+        with (
+            patch(
+                "app.services.assessment.api.callbacks.get_webhook_secret",
+                return_value=None,
+            ),
+            patch(
+                "app.services.assessment.api.callbacks.presign_result_files",
+                return_value=None,
+            ) as presign,
+            patch(
+                "app.services.assessment.api.callbacks.send_callback",
+                return_value=True,
+            ) as send_callback,
+        ):
+            sent = deliver(
+                session=db,
+                assessment=assessment,
+                result=_result(),
+                callback_url="https://client.example/webhook",
+                request_metadata=None,
+                failure_message=None,
+            )
+
+        assert sent is True
+        presign.assert_called_once_with(session=db, assessment=assessment)
+        assert send_callback.call_args.args[1]["metadata"] is None
+
+    def test_a_metadata_bug_still_delivers_the_result(self, db) -> None:
+        auth = get_user_test_auth_context(db)
+        assessment, _ = _seed(db, auth)
+
+        with (
+            patch(
+                "app.services.assessment.api.callbacks.get_webhook_secret",
+                return_value=None,
+            ),
+            patch(
+                "app.services.assessment.api.callbacks.presign_result_files",
+                side_effect=RuntimeError("s3 unreachable"),
+            ),
+            patch(
+                "app.services.assessment.api.callbacks.send_callback",
+                return_value=True,
+            ) as send_callback,
+        ):
+            sent = deliver(
+                session=db,
+                assessment=assessment,
+                result=_result(),
+                callback_url="https://client.example/webhook",
+                request_metadata=None,
+                failure_message=None,
+            )
+
+        assert sent is True
+        assert send_callback.call_args.args[1]["metadata"] is None