diff --git a/backend/app/crud/assessment/batch.py b/backend/app/crud/assessment/batch.py index d1014d414..a9314cf3f 100644 --- a/backend/app/crud/assessment/batch.py +++ b/backend/app/crud/assessment/batch.py @@ -25,15 +25,12 @@ AssessmentSubmission, ) from app.models.batch_job import BatchJob, BatchJobType -from app.models.config.assessment_blob import VIDEO_COLUMN_TYPE from app.models.llm.constants import DEFAULT_ASSESSMENT_BATCH_MAX_TOKENS from app.models.llm.request import ConfigBlob from app.services.assessment.utils.attachments import ( - attachment_type_for_row, build_anthropic_attachment_parts, build_gemini_attachment_parts, resolve_attachment_values, - resolve_item_type, rewrite_gcs_attachment_urls, ) from app.services.assessment.validators import ( @@ -130,13 +127,7 @@ def build_openai_jsonl( # Attachments for att in attachments: cell_value = row.get(att.column, "") - input_parts.extend( - resolve_attachment_values( - cell_value, - att, - type_override=attachment_type_for_row(att, row), - ) - ) + input_parts.extend(resolve_attachment_values(cell_value, att)) if not input_parts: logger.warning("[build_openai_jsonl] Skipping empty row | idx=%s", idx) @@ -195,17 +186,11 @@ def build_google_jsonl( has_video = False for att in attachments: cell_value = row.get(att.column, "") - type_override = attachment_type_for_row(att, row) - if cell_value.strip() and ( - resolve_item_type(att.type, type_override) == VIDEO_COLUMN_TYPE - ): + if cell_value.strip() and att.type == "video": has_video = True parts.extend( build_gemini_attachment_parts( - cell_value, - att, - type_override=type_override, - video_part_config=video_part_config, + cell_value, att, video_part_config=video_part_config ) ) @@ -282,13 +267,7 @@ def build_anthropic_jsonl( # Attachments (Anthropic uses url-source image/document blocks) for att in attachments: cell_value = row.get(att.column, "") - content.extend( - build_anthropic_attachment_parts( - cell_value, - att, - type_override=attachment_type_for_row(att, row), - ) - ) + content.extend(build_anthropic_attachment_parts(cell_value, att)) if not content: logger.warning("[build_anthropic_jsonl] Skipping empty row | idx=%s", idx) diff --git a/backend/app/models/assessment/assessment.py b/backend/app/models/assessment/assessment.py index 259f975ad..09aa9d7bf 100644 --- a/backend/app/models/assessment/assessment.py +++ b/backend/app/models/assessment/assessment.py @@ -9,7 +9,7 @@ from typing import TYPE_CHECKING, Any, Literal, Optional from uuid import UUID, uuid4 -from pydantic import BaseModel, Field, model_validator +from pydantic import BaseModel, Field from sqlalchemy import Column, Index, Text, text from sqlalchemy.dialects import postgresql from sqlalchemy.dialects.postgresql import JSONB @@ -84,30 +84,10 @@ class AssessmentAttachment(BaseModel): """External-dataset attachment column config (RUN / BATCH-by-ref).""" column: str = Field(..., description="Dataset column holding the attachment") - type: AttachmentColumnType | Literal["mixed"] = Field( - ..., - description=( - "'image'/'pdf'/'video' fix the type; 'mixed' resolves per-row via type_column" - ), + type: AttachmentColumnType = Field( + ..., description="'image', 'pdf', 'video' or 'audio'" ) format: Literal["url", "base64"] = Field(..., description="Data format") - type_column: str | None = Field( - None, description="'mixed' only: column whose value decides each row's type" - ) - type_value_map: dict[str, AttachmentColumnType] | None = Field( - None, - description="'mixed' only: maps a type_column value to 'image', 'pdf' or 'video'", - ) - - @model_validator(mode="after") - def _validate_mixed(self) -> "AssessmentAttachment": - if self.type == "mixed" and ( - self.type_column is None or not self.type_value_map - ): - raise ValueError( - "type='mixed' requires 'type_column' and a non-empty 'type_value_map'." - ) - return self # NOTE: Only used for UI. The API-client Assessment pipeline does not use this. diff --git a/backend/app/models/config/assessment_blob.py b/backend/app/models/config/assessment_blob.py index 8f6aed712..a5d82468f 100644 --- a/backend/app/models/config/assessment_blob.py +++ b/backend/app/models/config/assessment_blob.py @@ -1,6 +1,6 @@ import logging import re -from typing import Literal +from typing import Literal, get_args from pydantic import JsonValue, model_validator from sqlmodel import Field, SQLModel @@ -18,14 +18,15 @@ # object-typed dict. Provider strict-mode normalisation is a run-mode concern. JSON_SCHEMA_OBJECT_TYPE = "object" -IMAGE_COLUMN_TYPE = "image" -PDF_COLUMN_TYPE = "pdf" -VIDEO_COLUMN_TYPE = "video" +AttachmentColumnType = Literal["image", "pdf", "video", "audio"] +ColumnType = Literal["text"] | AttachmentColumnType # Input-column types carrying a media reference rather than prompt text. -ATTACHMENT_COLUMN_TYPES = (IMAGE_COLUMN_TYPE, PDF_COLUMN_TYPE, VIDEO_COLUMN_TYPE) +ATTACHMENT_COLUMN_TYPES = get_args(AttachmentColumnType) -AttachmentColumnType = Literal["image", "pdf", "video"] -ColumnType = Literal["text"] | AttachmentColumnType +# Media column types a provider cannot take, so a config pairing them is rejected at save. +UNSUPPORTED_COLUMN_TYPES_BY_PROVIDER: dict[str, tuple[str, ...]] = dict.fromkeys( + (Provider.OPENAI, Provider.ANTHROPIC), ("video", "audio") +) # {column} placeholders in a submission template; the capture group is the column name. PLACEHOLDER_RE = re.compile(r"\{(\w+)\}") @@ -51,8 +52,8 @@ class InputColumn(SQLModel): @model_validator(mode="after") def _validate_video_format(self): - if self.type == VIDEO_COLUMN_TYPE and self.format == "base64": - raise ValueError("A 'video' input column must be url-format.") + if self.type in ("video", "audio") and self.format == "base64": + raise ValueError(f"A '{self.type}' input column must be url-format.") return self @@ -225,3 +226,26 @@ def _validate_submission_placeholders(self): f"{', '.join(unknown)}" ) return self + + @model_validator(mode="after") + def _validate_column_types_supported(self): + """Every stage's provider must accept each media column type in input_schema.""" + stage_providers = {"assessment": self.assessment.provider} + if self.pre_filters and self.pre_filters.topic_relevance: + stage_providers[ + "pre_filter topic_relevance" + ] = self.pre_filters.topic_relevance.provider + + for stage, provider in stage_providers.items(): + unsupported = UNSUPPORTED_COLUMN_TYPES_BY_PROVIDER.get(provider, ()) + offending = sorted( + column + for column, spec in self.input_schema.items() + if spec.type in unsupported + ) + if offending: + raise ValueError( + f"{stage} provider '{provider}' does not support " + f"{'/'.join(unsupported)} input columns: {', '.join(offending)}" + ) + return self diff --git a/backend/app/services/assessment/api/batch.py b/backend/app/services/assessment/api/batch.py index 0e760b63e..4bb4ce5eb 100644 --- a/backend/app/services/assessment/api/batch.py +++ b/backend/app/services/assessment/api/batch.py @@ -207,7 +207,7 @@ def column_kinds( ) -> tuple[list[str], list[AssessmentAttachment]]: """Split columns into text names and attachment specs per the config's ``input_schema``. - A column typed image/pdf/video is an attachment (url-format only); anything else is text. + A column typed image/pdf/video/audio is an attachment (url-format only); anything else is text. """ text_columns: list[str] = [] attachments: list[AssessmentAttachment] = [] diff --git a/backend/app/services/assessment/api/submission.py b/backend/app/services/assessment/api/submission.py index c8a3a934a..f038d9603 100644 --- a/backend/app/services/assessment/api/submission.py +++ b/backend/app/services/assessment/api/submission.py @@ -6,7 +6,6 @@ """ import logging -from urllib.parse import urlparse from uuid import UUID, uuid4 from asgi_correlation_id import correlation_id @@ -36,40 +35,16 @@ open_uploaded_rows, upload_submission_rows, ) +from app.services.assessment.utils.attachments import ( + attachment_type_from_url, + split_attachment_urls, +) from app.utils import validate_callback_url logger = logging.getLogger(__name__) # Attachment cell values are provided as URLs (base64 is unsupported for batch). _URL_PREFIXES = ("http://", "https://", "gs://") -_EXTENSION_TYPES = { - ".pdf": "pdf", - ".png": "image", - ".jpg": "image", - ".jpeg": "image", - ".gif": "image", - ".webp": "image", - ".mp4": "video", - ".mov": "video", - ".mpeg": "video", - ".mpg": "video", - ".avi": "video", - ".wmv": "video", - ".flv": "video", - ".webm": "video", - ".3gp": "video", -} - - -def _url_attachment_type(url: str) -> str | None: - """The kind a URL's own extension implies, or None when it names no extension. - - Presigned URLs carry a query string, so only the path is read. - """ - path = urlparse(url).path.lower() - return next( - (kind for ext, kind in _EXTENSION_TYPES.items() if path.endswith(ext)), None - ) def _validate_rows_against_schema( @@ -104,8 +79,11 @@ def _validate_rows_against_schema( for column, spec in input_schema.items(): column_type = (spec or {}).get("type") value = (row.get(column) or "").strip() - if value and column_type in ATTACHMENT_COLUMN_TYPES: - if not value.startswith(_URL_PREFIXES): + if not value or column_type not in ATTACHMENT_COLUMN_TYPES: + continue + # A cell may hold several URLs; the builder sends each one separately. + for url in split_attachment_urls(value): + if not url.startswith(_URL_PREFIXES): raise HTTPException( status_code=422, detail=( @@ -114,7 +92,7 @@ def _validate_rows_against_schema( ), ) # The provider only reports a mistyped column later, as an opaque format error. - actual = _url_attachment_type(value) + actual = attachment_type_from_url(url) if actual is not None and actual != column_type: raise HTTPException( status_code=422, diff --git a/backend/app/services/assessment/prefilter/topic_relevance.py b/backend/app/services/assessment/prefilter/topic_relevance.py index 1fa6acb43..2b53b329c 100644 --- a/backend/app/services/assessment/prefilter/topic_relevance.py +++ b/backend/app/services/assessment/prefilter/topic_relevance.py @@ -12,7 +12,6 @@ from app.services.assessment.prefilter import constants from app.services.assessment.prefilter.request_builder import build_request_line from app.services.assessment.utils.attachments import ( - attachment_type_for_row, build_gemini_attachment_parts, resolve_attachment_values, ) @@ -65,11 +64,10 @@ def build_topic_relevance_requests( cell = row.get(att.column, "") if not cell.strip(): continue - override = attachment_type_for_row(att, row) attachment_parts.extend( - resolve_attachment_values(cell, att, type_override=override) + resolve_attachment_values(cell, att) if is_openai - else build_gemini_attachment_parts(cell, att, type_override=override) + else build_gemini_attachment_parts(cell, att) ) lines.append( build_request_line( diff --git a/backend/app/services/assessment/service.py b/backend/app/services/assessment/service.py index fb2286b29..c7fff7f4c 100644 --- a/backend/app/services/assessment/service.py +++ b/backend/app/services/assessment/service.py @@ -30,6 +30,7 @@ InputBinding, StageStatus, ) +from app.models.config.assessment_blob import UNSUPPORTED_COLUMN_TYPES_BY_PROVIDER from app.models.config.config import ConfigTag from app.services.llm.providers.registry import LLMProvider @@ -173,6 +174,23 @@ def start_assessment( f"Supported providers: {sorted(_SUPPORTED_BATCH_PROVIDERS)}" ), ) + unsupported_types = UNSUPPORTED_COLUMN_TYPES_BY_PROVIDER.get( + str(provider).removesuffix("-native"), () + ) + unsupported_columns = sorted( + att.column + for att in request.input_binding.attachments + if att.type in unsupported_types + ) + if unsupported_columns: + raise HTTPException( + status_code=422, + detail=( + f"Config {cfg.id} v{cfg.version} uses provider '{provider}', " + f"which does not support {'/'.join(unsupported_types)} " + f"attachments: {', '.join(unsupported_columns)}" + ), + ) resolved_configs.append(cfg) assessment = create_assessment( diff --git a/backend/app/services/assessment/utils/attachments.py b/backend/app/services/assessment/utils/attachments.py index c47724cd5..62d685434 100644 --- a/backend/app/services/assessment/utils/attachments.py +++ b/backend/app/services/assessment/utils/attachments.py @@ -10,11 +10,6 @@ from app.core.config import settings from app.models.assessment import AssessmentAttachment -from app.models.config.assessment_blob import ( - ATTACHMENT_COLUMN_TYPES, - IMAGE_COLUMN_TYPE, - VIDEO_COLUMN_TYPE, -) from app.models.llm.constants import KaapiProvider from app.services.buckets.attachments import is_gcs_uri, resolve_attachments @@ -45,13 +40,42 @@ ".3gp": "video/3gpp", } +_AUDIO_MIME_BY_EXT = { + ".wav": "audio/wav", + ".mp3": "audio/mp3", + ".aiff": "audio/aiff", + ".aac": "audio/aac", + ".ogg": "audio/ogg", + ".flac": "audio/flac", + ".m4a": "audio/m4a", +} + _PDF_MIME = "application/pdf" _DEFAULT_IMAGE_MIME = "image/png" _DEFAULT_VIDEO_MIME = "video/mp4" +_DEFAULT_AUDIO_MIME = "audio/mp3" YOUTUBE_HOSTS = ("youtube.com", "www.youtube.com", "youtu.be", "m.youtube.com") +def attachment_type_from_url(url: str) -> str | None: + """The attachment type a URL's own extension implies, or None when it names none. + + Presigned URLs carry a query string, so only the path is read. + """ + path = urlparse(url).path.lower() + if path.endswith(".pdf"): + return "pdf" + for kind, mime_by_ext in ( + ("image", _IMAGE_MIME_BY_EXT), + ("video", _VIDEO_MIME_BY_EXT), + ("audio", _AUDIO_MIME_BY_EXT), + ): + if path.endswith(tuple(mime_by_ext)): + return kind + return None + + def split_attachment_urls(value: str) -> list[str]: """Split comma/newline separated attachment URLs from a single dataset cell.""" return [part.strip() for part in re.split(r"[\n,]+", value) if part.strip()] @@ -157,82 +181,31 @@ def _resolve_video_mime_from_url(url: str) -> str | None: return None -def resolve_item_type(declared: str, type_override: str | None = None) -> str | None: - """Resolve an attachment item as 'image', 'pdf' or 'video' from the declared type. - - A per-row ``type_override`` (for 'mixed' columns) wins, else the column's declared - ``type``. Returns None when the type stays unresolved (e.g. a 'mixed' row whose - value didn't map to a concrete type) so callers can skip rather than guess. - """ - item_type = type_override or declared - return item_type if item_type in ATTACHMENT_COLUMN_TYPES else None - - -def _normalize_type_value(value: str) -> str: - return re.sub(r"\s+", " ", value).strip().casefold() - - -def _split_type_values(value: str) -> list[str]: - return [ - normalized - for part in re.split(r"[\n,]+", value) - if (normalized := _normalize_type_value(part)) - ] - - -def attachment_type_for_row( - att: AssessmentAttachment, row: dict[str, str] -) -> str | None: - """For a 'mixed' column, resolve this row's type from type_column + type_value_map. - - Returns a concrete attachment type, or None to let normal detection decide. - """ - type_column = getattr(att, "type_column", None) - type_value_map = getattr(att, "type_value_map", None) - if att.type != "mixed" or not type_column or not type_value_map: - return None - - normalized_map: dict[str, str] = {} - for raw_values, mapped_type in type_value_map.items(): - if mapped_type not in ATTACHMENT_COLUMN_TYPES: - continue - for value in _split_type_values(raw_values): - normalized_map[value] = mapped_type - - row_values = _split_type_values(row.get(type_column) or "") - if not row_values: - return None - - mapped_values = { - normalized_map[value] for value in row_values if value in normalized_map - } - return mapped_values.pop() if len(mapped_values) == 1 else None +def _resolve_audio_mime_from_url(url: str) -> str | None: + path = urlparse(url).path or "" + for ext, mime in _AUDIO_MIME_BY_EXT.items(): + if path.lower().endswith(ext): + return mime + return None def resolve_attachment_values( value: str, att: AssessmentAttachment, - type_override: str | None = None, ) -> list[dict[str, JsonValue]]: """Resolve one dataset cell into OpenAI-supported content params (by URL).""" value = value.strip() if not value: return [] - item_type = resolve_item_type(att.type, type_override) - if item_type is None: - logger.warning( - "[resolve_attachment_values] Unresolved type for column=%s — skipping", - att.column, - ) - return [] - # Openai doesn't support video attachments - if item_type == VIDEO_COLUMN_TYPE: + item_type = att.type + # Openai doesn't support video or audio attachments (Responses API) + if item_type in ("video", "audio"): return [] resolved: list[dict[str, JsonValue]] = [] for item_value in split_attachment_urls(value): url = to_direct_attachment_url(item_value, item_type) - if item_type == IMAGE_COLUMN_TYPE: + if item_type == "image": resolved.append({"type": "input_image", "image_url": url}) else: resolved.append({"type": "input_file", "file_url": url}) @@ -242,27 +215,20 @@ def resolve_attachment_values( def build_anthropic_attachment_parts( value: str, att: AssessmentAttachment, - type_override: str | None = None, ) -> list[dict[str, JsonValue]]: """Convert one dataset cell into one or more Anthropic content blocks (by URL).""" value = value.strip() if not value: return [] - item_type = resolve_item_type(att.type, type_override) - if item_type is None: - logger.warning( - "[build_anthropic_attachment_parts] Unresolved type for column=%s — skipping", - att.column, - ) - return [] - # Anthropic doesn't support video attachments - if item_type == VIDEO_COLUMN_TYPE: + item_type = att.type + # Anthropic doesn't support video or audio attachments + if item_type in ("video", "audio"): return [] blocks: list[dict[str, JsonValue]] = [] for item_value in split_attachment_urls(value): url = to_direct_attachment_url(item_value, item_type) - if item_type == IMAGE_COLUMN_TYPE: + if item_type == "image": blocks.append({"type": "image", "source": {"type": "url", "url": url}}) else: blocks.append({"type": "document", "source": {"type": "url", "url": url}}) @@ -282,34 +248,30 @@ def build_gemini_video_part( def build_gemini_attachment_parts( value: str, att: AssessmentAttachment, - type_override: str | None = None, video_part_config: dict[str, JsonValue] | None = None, ) -> list[dict[str, JsonValue]]: """Convert one dataset cell into one or more Gemini content parts (by URL). Mirrors the per-item type routing used for the L2 batch so the same - image/pdf/video handling applies to prefilter (topic relevance) calls. + image/pdf/video/audio handling applies to prefilter (topic relevance) calls. ``video_part_config`` comes from the Google mapper, already wire-shaped. """ value = value.strip() if not value: return [] - item_type = resolve_item_type(att.type, type_override) - if item_type is None: - logger.warning( - "[build_gemini_attachment_parts] Unresolved type for column=%s — skipping", - att.column, - ) - return [] + item_type = att.type parts: list[dict[str, JsonValue]] = [] for item_value in split_attachment_urls(value): url = to_direct_attachment_url(item_value, item_type) - if item_type == IMAGE_COLUMN_TYPE: + if item_type == "image": mime_type = _guess_image_mime_from_url(url) or _DEFAULT_IMAGE_MIME parts.append({"fileData": {"mimeType": mime_type, "fileUri": url}}) - elif item_type == VIDEO_COLUMN_TYPE: + elif item_type == "video": parts.append(build_gemini_video_part(url, video_part_config)) + elif item_type == "audio": + mime_type = _resolve_audio_mime_from_url(url) or _DEFAULT_AUDIO_MIME + parts.append({"fileData": {"mimeType": mime_type, "fileUri": url}}) else: parts.append({"fileData": {"mimeType": _PDF_MIME, "fileUri": url}}) return parts diff --git a/backend/app/tests/assessment/test_api_crud.py b/backend/app/tests/assessment/test_api_crud.py index b49ce63c2..a4dd0bbda 100644 --- a/backend/app/tests/assessment/test_api_crud.py +++ b/backend/app/tests/assessment/test_api_crud.py @@ -407,6 +407,57 @@ def _blob_with_submissions( return blob +class TestUnsupportedColumnTypeValidator: + @pytest.mark.parametrize("provider", ["openai", "anthropic"]) + @pytest.mark.parametrize("column_type", ["audio", "video"]) + def test_media_column_rejected_for_unsupported_assessment_provider( + self, provider: str, column_type: str + ) -> None: + blob = _blob_with_submissions( + input_schema={"a": {"type": "text"}, "m": {"type": column_type}} + ) + blob["assessment"]["provider"] = provider + with pytest.raises( + ValidationError, match=f"assessment provider '{provider}'.*: m" + ): + AssessmentConfigBlob.model_validate(blob) + + def test_media_column_rejected_for_unsupported_prefilter_provider(self) -> None: + blob = _blob_with_submissions( + input_schema={"a": {"type": "text"}, "m": {"type": "audio"}}, + topic_submission="rank {a}", + ) + blob["assessment"]["provider"] = "google-gcp" + with pytest.raises( + ValidationError, match="pre_filter topic_relevance provider 'openai'" + ): + AssessmentConfigBlob.model_validate(blob) + + @pytest.mark.parametrize("provider", ["google", "google-aistudio", "google-gcp"]) + def test_media_column_accepted_for_google(self, provider: str) -> None: + blob = _blob_with_submissions( + input_schema={ + "a": {"type": "text"}, + "m": {"type": "audio"}, + "v": {"type": "video"}, + } + ) + blob["assessment"]["provider"] = provider + assert ( + AssessmentConfigBlob.model_validate(blob).input_schema["m"].type == "audio" + ) + + def test_image_and_pdf_columns_accepted_for_openai(self) -> None: + blob = _blob_with_submissions( + input_schema={ + "a": {"type": "text"}, + "i": {"type": "image"}, + "p": {"type": "pdf"}, + } + ) + AssessmentConfigBlob.model_validate(blob) + + class TestSubmissionPlaceholderValidator: def test_valid_assessment_and_prefilter_submission_passes(self) -> None: blob = AssessmentConfigBlob.model_validate( @@ -500,6 +551,14 @@ def test_video_base64_rejected(self) -> None: with pytest.raises(ValidationError): InputColumn.model_validate({"type": "video", "format": "base64"}) + def test_audio_base64_rejected(self) -> None: + with pytest.raises(ValidationError, match="'audio' input column must be url"): + InputColumn.model_validate({"type": "audio", "format": "base64"}) + + def test_audio_url_accepted(self) -> None: + col = InputColumn.model_validate({"type": "audio", "format": "url"}) + assert col.type == "audio" + def test_video_url_accepted(self) -> None: col = InputColumn.model_validate({"type": "video", "format": "url"}) assert col.type == "video" diff --git a/backend/app/tests/assessment/test_api_submission.py b/backend/app/tests/assessment/test_api_submission.py index adecc8900..57d748034 100644 --- a/backend/app/tests/assessment/test_api_submission.py +++ b/backend/app/tests/assessment/test_api_submission.py @@ -208,6 +208,51 @@ def test_gs_uri_attachment_value_passes_validation(self) -> None: {"img": {"type": "image", "format": "url"}}, ) + def test_audio_column_with_audio_url_passes_validation(self) -> None: + submission._validate_rows_against_schema( + [{"clip": "https://x.com/a.wav?sig=1"}], + {"clip": {"type": "audio", "format": "url"}}, + ) + + def test_audio_column_with_video_url_is_422(self) -> None: + with pytest.raises(HTTPException) as exc: + submission._validate_rows_against_schema( + [{"clip": "https://x.com/a.mp4"}], + {"clip": {"type": "audio", "format": "url"}}, + ) + assert exc.value.status_code == 422 + assert "Change the column type to 'video'" in exc.value.detail + + @pytest.mark.parametrize( + "cell", + [ + "https://x.com/a.mp4,https://x.com/b.wav", + "https://x.com/a.wav,https://x.com/b.mp4", + "https://x.com/a.mp4\nhttps://x.com/b.wav", + ], + ) + def test_multi_url_cell_with_mistyped_url_is_422(self, cell: str) -> None: + with pytest.raises(HTTPException) as exc: + submission._validate_rows_against_schema( + [{"clip": cell}], {"clip": {"type": "audio", "format": "url"}} + ) + assert exc.value.status_code == 422 + assert "points to a 'video'" in exc.value.detail + + def test_multi_url_cell_with_non_url_part_is_422(self) -> None: + with pytest.raises(HTTPException) as exc: + submission._validate_rows_against_schema( + [{"clip": "https://x.com/a.wav, not-a-url"}], + {"clip": {"type": "audio", "format": "url"}}, + ) + assert "must be a URL" in exc.value.detail + + def test_multi_url_cell_with_all_audio_urls_passes(self) -> None: + submission._validate_rows_against_schema( + [{"clip": "https://x.com/a.wav, gs://b/c.mp3\nhttps://x.com/d.m4a?sig=1"}], + {"clip": {"type": "audio", "format": "url"}}, + ) + def test_unsupported_provider_is_422(self, db) -> None: auth = get_user_test_auth_context(db) # Build a config whose stored blob names an unsupported batch provider. diff --git a/backend/app/tests/assessment/test_batch.py b/backend/app/tests/assessment/test_batch.py index cfeb8551d..9128f8c1a 100644 --- a/backend/app/tests/assessment/test_batch.py +++ b/backend/app/tests/assessment/test_batch.py @@ -20,11 +20,10 @@ from app.models.llm.constants import DEFAULT_ASSESSMENT_BATCH_MAX_TOKENS from app.services.assessment.utils.attachments import ( _guess_image_mime_from_url, - attachment_type_for_row, + attachment_type_from_url, build_anthropic_attachment_parts, build_gemini_attachment_parts, resolve_attachment_values, - resolve_item_type, rewrite_gcs_attachment_urls, split_attachment_urls, to_direct_attachment_url, @@ -607,24 +606,8 @@ def test_build_anthropic_attachment_parts(self) -> None: assert parts[0]["source"] == {"type": "url", "url": "https://example.com/a.png"} -class TestResolveItemType: - """Image/pdf routing now trusts the user-declared type (no detection).""" - - def test_declared_image(self) -> None: - assert resolve_item_type("image") == "image" - - def test_declared_pdf(self) -> None: - assert resolve_item_type("pdf") == "pdf" - - def test_override_wins(self) -> None: - assert resolve_item_type("image", "pdf") == "pdf" - assert resolve_item_type("pdf", "image") == "image" - - def test_mixed_without_override_is_unresolved(self) -> None: - assert resolve_item_type("mixed") is None - - def test_unknown_declared_is_unresolved(self) -> None: - assert resolve_item_type("whatever") is None +class TestDeclaredAttachmentType: + """Routing trusts the user-declared column type (no detection).""" def test_column_uses_single_declared_type(self) -> None: """One column, many URLs -> all routed by the declared type.""" @@ -635,6 +618,21 @@ def test_column_uses_single_declared_type(self) -> None: assert types == ["input_file", "input_file"] +class TestAttachmentTypeFromUrl: + @pytest.mark.parametrize( + ("url", "expected"), + [ + ("https://x.com/a.PDF?sig=1", "pdf"), + ("https://x.com/a.png", "image"), + ("https://x.com/a.mp4", "video"), + ("https://x.com/a.m4a", "audio"), + ("https://x.com/a", None), + ], + ) + def test_type_from_extension(self, url: str, expected: str | None) -> None: + assert attachment_type_from_url(url) == expected + + class TestAttachmentMime: def test_guess_image_mime_from_url_variants(self) -> None: assert _guess_image_mime_from_url("http://x/a.PNG") == "image/png" @@ -643,126 +641,16 @@ def test_guess_image_mime_from_url_variants(self) -> None: assert _guess_image_mime_from_url("http://x/a.txt") is None -class TestAttachmentTypeForRow: - def test_mixed_resolves_from_type_column(self) -> None: - from app.services.assessment.utils.attachments import attachment_type_for_row - - att = AssessmentAttachment( - column="Docs", - type="mixed", - format="url", - type_column="DOC type", - type_value_map={"Photo": "image", "Report": "pdf"}, - ) - assert attachment_type_for_row(att, {"DOC type": "Photo"}) == "image" - assert attachment_type_for_row(att, {"DOC type": "Report"}) == "pdf" - assert attachment_type_for_row(att, {"DOC type": "Unknown"}) is None - - def test_mixed_resolves_comma_separated_value_lists(self) -> None: - from app.services.assessment.utils.attachments import attachment_type_for_row - - att = AssessmentAttachment( - column="Docs", - type="mixed", - format="url", - type_column="DOC type", - type_value_map={"Img-Prototype, Img-Handtext": "image", "Pdf": "pdf"}, - ) - - assert attachment_type_for_row(att, {"DOC type": "Img-Prototype"}) == "image" - assert attachment_type_for_row(att, {"DOC type": "Img-Handtext"}) == "image" - assert attachment_type_for_row(att, {"DOC type": "pdf"}) == "pdf" - - def test_mixed_resolves_row_value_lists_when_same_type(self) -> None: - from app.services.assessment.utils.attachments import attachment_type_for_row - - att = AssessmentAttachment( - column="Docs", - type="mixed", - format="url", - type_column="DOC type", - type_value_map={"Img-Prototype, Img-Handtext": "image", "Pdf": "pdf"}, - ) - - assert ( - attachment_type_for_row( - att, - {"DOC type": "Img-Prototype, Img-Handtext"}, - ) - == "image" - ) - assert attachment_type_for_row(att, {"DOC type": "Img-Prototype, Pdf"}) is None - - def test_mixed_missing_type_mapping_fields_returns_none(self) -> None: - from app.services.assessment.utils.attachments import attachment_type_for_row - - att = SimpleNamespace(column="Docs", type="mixed", format="url") - - assert attachment_type_for_row(att, {"Docs": "x"}) is None - - def test_non_mixed_returns_none(self) -> None: - from app.services.assessment.utils.attachments import attachment_type_for_row - - att = AssessmentAttachment(column="Docs", type="image", format="url") - assert attachment_type_for_row(att, {"Docs": "x"}) is None - - def test_mixed_config_missing_routing_fields_is_rejected(self) -> None: - import pytest - from pydantic import ValidationError - - with pytest.raises(ValidationError): - AssessmentAttachment(column="Docs", type="mixed", format="url") - - def test_mixed_config_invalid_map_value_is_rejected(self) -> None: - import pytest - from pydantic import ValidationError - - with pytest.raises(ValidationError): - AssessmentAttachment( - column="Docs", - type="mixed", - format="url", - type_column="DOC type", - type_value_map={"Report": "spreadsheet"}, - ) - - def test_override_forces_part_type(self) -> None: - from app.services.assessment.utils.attachments import resolve_attachment_values - - att = AssessmentAttachment( - column="Docs", - type="mixed", - format="url", - type_column="DOC type", - type_value_map={"Report": "pdf"}, - ) - url = "https://drive.google.com/file/d/ID/view" - parts = resolve_attachment_values(url, att, type_override="pdf") - assert parts[0]["type"] == "input_file" - - class TestAttachmentResolutionBranches: _IMG = AssessmentAttachment(column="Docs", type="image", format="url") _PDF = AssessmentAttachment(column="Docs", type="pdf", format="url") _VIDEO = AssessmentAttachment(column="Docs", type="video", format="url") - _MIXED = AssessmentAttachment( - column="Docs", - type="mixed", - format="url", - type_column="DOC type", - type_value_map={"Report": "pdf"}, - ) + _AUDIO = AssessmentAttachment(column="Docs", type="audio", format="url") def test_blank_value_returns_empty(self) -> None: assert resolve_attachment_values(" ", self._IMG) == [] assert build_gemini_attachment_parts(" ", self._IMG) == [] - def test_unresolved_mixed_is_skipped(self) -> None: - url = "https://x.com/a.jpg" - # No override and declared 'mixed' -> unresolved -> skip rather than guess. - assert resolve_attachment_values(url, self._MIXED) == [] - assert build_gemini_attachment_parts(url, self._MIXED) == [] - def test_gemini_image_and_pdf_parts(self) -> None: img = build_gemini_attachment_parts("https://x.com/a.png", self._IMG)[0] pdf = build_gemini_attachment_parts("https://x.com/a.pdf", self._PDF)[0] @@ -774,6 +662,42 @@ def test_openai_and_anthropic_skip_video(self) -> None: assert resolve_attachment_values(url, self._VIDEO) == [] assert build_anthropic_attachment_parts(url, self._VIDEO) == [] + def test_openai_and_anthropic_skip_audio(self) -> None: + url = "https://x.com/a.wav" + assert resolve_attachment_values(url, self._AUDIO) == [] + assert build_anthropic_attachment_parts(url, self._AUDIO) == [] + + def test_gemini_audio_part_resolves_mime_from_extension(self) -> None: + part = build_gemini_attachment_parts( + "https://x.com/a.WAV?sig=1, gs://b/c.flac", self._AUDIO + ) + assert part[0]["fileData"] == { + "fileUri": "https://x.com/a.WAV?sig=1", + "mimeType": "audio/wav", + } + assert part[1]["fileData"]["mimeType"] == "audio/flac" + + def test_gemini_audio_part_m4a_mime(self) -> None: + part = build_gemini_attachment_parts("https://x.com/a.m4a", self._AUDIO)[0] + assert part["fileData"]["mimeType"] == "audio/m4a" + + def test_gemini_audio_part_unknown_extension_uses_default_mime(self) -> None: + part = build_gemini_attachment_parts("https://x.com/clip", self._AUDIO)[0] + assert part["fileData"]["mimeType"] == "audio/mp3" + + def test_google_jsonl_carries_audio_part(self) -> None: + jsonl = build_google_jsonl( + rows=[{"clip": "https://x.com/a.mp3", "q": "hi"}], + text_columns=["q"], + attachments=[ + AssessmentAttachment(column="clip", type="audio", format="url") + ], + prompt_template="{q}", + google_params={"model": "gemini-2.5-flash"}, + ) + parts = jsonl[0]["request"]["contents"][0]["parts"] + assert parts[1]["fileData"]["mimeType"] == "audio/mp3" + def test_gemini_video_part_youtube_omits_mime_type(self) -> None: part = build_gemini_attachment_parts( "https://youtu.be/gKJiCGNaAwg", self._VIDEO @@ -791,16 +715,3 @@ def test_gemini_video_part_direct_url_resolves_mime_and_config(self) -> None: "mimeType": "video/mp4", } assert part["videoMetadata"] == {"fps": 1.0} - - def test_type_for_row_blank_value_returns_none(self) -> None: - assert attachment_type_for_row(self._MIXED, {"DOC type": " "}) is None - - def test_type_for_row_ignores_invalid_map_value(self) -> None: - # SimpleNamespace bypasses the model validator to exercise the guard that - # skips map entries whose target type isn't 'image'/'pdf'. - att = SimpleNamespace( - type="mixed", - type_column="DOC type", - type_value_map={"Report": "spreadsheet"}, - ) - assert attachment_type_for_row(att, {"DOC type": "Report"}) is None diff --git a/backend/app/tests/assessment/test_service.py b/backend/app/tests/assessment/test_service.py index 6b0508cd6..75784e982 100644 --- a/backend/app/tests/assessment/test_service.py +++ b/backend/app/tests/assessment/test_service.py @@ -10,6 +10,7 @@ from app.crud.assessment import core as assessment_core from app.crud.assessment.core import _read_exec from app.models.assessment import ( + AssessmentAttachment, AssessmentConfigRef, AssessmentRunCreate, AssessmentStatus, @@ -146,6 +147,42 @@ def test_rejects_unsupported_provider(self) -> None: ) create_assessment.assert_not_called() + @pytest.mark.parametrize("provider", ["openai", "openai-native", "anthropic"]) + def test_rejects_audio_attachment_for_unsupported_provider( + self, provider: str + ) -> None: + request = _make_request(UUID("00000000-0000-0000-0000-000000000001")) + request.input_binding.attachments = [ + AssessmentAttachment(column="clip", type="audio", format="url") + ] + config_blob = SimpleNamespace(completion=SimpleNamespace(provider=provider)) + + with ( + patch( + "app.services.assessment.service.get_submission_by_id", + return_value=_make_dataset(), + ), + patch( + "app.services.assessment.service.resolve_evaluation_config", + return_value=(config_blob, None), + ), + patch( + "app.services.assessment.service.create_assessment" + ) as create_assessment, + _assessment_config_crud_patch(), + ): + with pytest.raises(HTTPException) as exc: + start_assessment( + session=MagicMock(), + request=request, + organization_id=1, + project_id=1, + ) + + assert exc.value.status_code == 422 + assert "clip" in exc.value.detail + create_assessment.assert_not_called() + def test_google_provider_is_supported(self) -> None: session = MagicMock() request = _make_request(UUID("00000000-0000-0000-0000-000000000001")) diff --git a/docs/wiki/modules/assessment.md b/docs/wiki/modules/assessment.md index 7abfeddac..655beb110 100644 --- a/docs/wiki/modules/assessment.md +++ b/docs/wiki/modules/assessment.md @@ -17,7 +17,7 @@ All paths relative to `backend/app/`. | `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; `strict: true` means the column must be present and non-blank in every submission row; it is **off by default** because the console declares every sheet column (the backend rejects undeclared ones) and real sheets have blanks — a blank on a non-strict column passes and `build_rows` fills omitted keys with `""` so the placeholder still resolves; note the config store persists the validated blob *with defaults*, so a saved version pins the default in force at save time; `InputColumn` is `extra=forbid`, so a misspelt flag fails at config save; attachment columns (`image`/`pdf`/`video`) are url-format only, and `video` is Google-only — the OpenAI and Anthropic part builders log and skip a video cell) — 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, projected onto the schema's columns (a sheet column `input_schema` does not name is dropped, so the schema is the selection and the console need only declare the `@`-referenced columns), 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`). +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; `strict: true` means the column must be present and non-blank in every submission row; it is **off by default** because the console declares every sheet column (the backend rejects undeclared ones) and real sheets have blanks — a blank on a non-strict column passes and `build_rows` fills omitted keys with `""` so the placeholder still resolves; note the config store persists the validated blob *with defaults*, so a saved version pins the default in force at save time; `InputColumn` is `extra=forbid`, so a misspelt flag fails at config save; attachment columns (`image`/`pdf`/`video`/`audio`) are url-format only, and `video`/`audio` are Google-only (AI Studio + GCP share `build_google_jsonl`) — a config whose assessment or pre-filter provider is `openai`/`anthropic` is rejected at save when `input_schema` declares one (`UNSUPPORTED_COLUMN_TYPES_BY_PROVIDER`), so no run reaches the builders with such a column) — 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, projected onto the schema's columns (a sheet column `input_schema` does not name is dropped, so the schema is the selection and the console need only declare the `@`-referenced columns), 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. 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` at submit, and the `open_submission_rows` context manager on read, which streams the JSONL line by line and closes the body on exit. Rows are read only inside `_submit_stage`, never on a poll, and only the rows the stage's subset needs are held (the count comes from `execution.total_items`, so an empty subset never opens the file). A storage read failure raises `SubmissionUnavailableError`, which requeues the task instead of failing the execution; a corrupt line is a `ValueError` and terminal. 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.