From 81e8cc69e53a4502e86f20adaefb7e2bbd2079f9 Mon Sep 17 00:00:00 2001 From: Vishakh Pillai Date: Mon, 20 Jul 2026 12:35:04 -0400 Subject: [PATCH 01/10] feat(api): validate ES uploads with bronze dataio converters and Pandera Load school-specific converters from bronze training_inputs/dataio.py under isolated module names, then validate via read_raw_es_* for parity with ES Databricks jobs. Soft-fallback when dataio is missing; fail closed on converter runtime errors. PDP and Legacy paths unchanged. Co-authored-by: Cursor --- src/webapp/databricks.py | 73 ++++ src/webapp/databricks_test.py | 37 ++ src/webapp/gcsutil.py | 4 +- src/webapp/routers/data.py | 6 +- src/webapp/routers/data_test.py | 6 +- src/webapp/validation.py | 372 +++++++++++++++++--- src/webapp/validation_es_test.py | 288 +++++++++++++++ src/webapp/validation_pdp_edvise.py | 16 +- src/webapp/validation_pdp_read_path_test.py | 92 +++-- src/webapp/validation_test.py | 89 +++-- 10 files changed, 842 insertions(+), 141 deletions(-) create mode 100644 src/webapp/validation_es_test.py diff --git a/src/webapp/databricks.py b/src/webapp/databricks.py index 538b9db0..3dff61ef 100644 --- a/src/webapp/databricks.py +++ b/src/webapp/databricks.py @@ -62,6 +62,9 @@ re.IGNORECASE, ) +# Only these basenames may be read from bronze_volume/training_inputs/ (path traversal safe). +ALLOWED_BRONZE_TRAINING_INPUTS_FILES = frozenset({"dataio.py"}) + def _create_databricks_workspace_client(operation: str) -> WorkspaceClient: """ @@ -640,6 +643,76 @@ def download_bronze_volume_file(self, inst_name: str, file_name: str) -> Any: raise ValueError("Databricks download returned no contents.") return stream + def download_bronze_training_inputs_file( + self, inst_name: str, relative_path: str = "dataio.py" + ) -> Any: + """ + Download a file from ``bronze_volume/training_inputs/`` for an institution. + + Only allowlisted basenames (currently ``dataio.py``) are permitted; nested + paths, ``..``, and absolute paths are rejected. + + Args: + inst_name: Institution display name (passed through ``databricksify_inst_name``). + relative_path: Basename under ``training_inputs/`` (default ``dataio.py``). + + Returns: + Byte stream of file contents (same shape as ``download_bronze_volume_file``). + + Raises: + ValueError: If path is unsafe, Databricks is not configured, or download fails. + """ + if not relative_path or not isinstance(relative_path, str): + raise ValueError("relative_path is required.") + if relative_path != os.path.basename(relative_path): + raise ValueError( + "relative_path must be a basename only (no directories or separators)." + ) + if relative_path in (".", "..") or ".." in relative_path: + raise ValueError("relative_path must not contain path traversal.") + if relative_path not in ALLOWED_BRONZE_TRAINING_INPUTS_FILES: + raise ValueError( + f"relative_path must be one of {sorted(ALLOWED_BRONZE_TRAINING_INPUTS_FILES)}." + ) + if not databricks_vars.get("DATABRICKS_HOST_URL") or not databricks_vars.get( + "CATALOG_NAME" + ): + raise ValueError("Databricks integration not configured.") + if not gcs_vars.get("GCP_SERVICE_ACCOUNT_EMAIL"): + raise ValueError("GCP service account email not configured.") + + try: + w = WorkspaceClient( + host=databricks_vars["DATABRICKS_HOST_URL"], + google_service_account=gcs_vars["GCP_SERVICE_ACCOUNT_EMAIL"], + ) + except Exception as e: + LOGGER.exception( + "Failed to create Databricks WorkspaceClient with host: %s and service account: %s", + databricks_vars.get("DATABRICKS_HOST_URL"), + gcs_vars.get("GCP_SERVICE_ACCOUNT_EMAIL"), + ) + raise ValueError(f"Workspace client creation failed: {e}") from e + + db_inst_name = databricksify_inst_name(inst_name) + volume_path = ( + f"/Volumes/{databricks_vars['CATALOG_NAME']}/" + f"{db_inst_name}_bronze/bronze_volume/training_inputs/{relative_path}" + ) + + try: + response = w.files.download(volume_path) + except Exception as e: + LOGGER.exception("Failed to download from %s", volume_path) + raise ValueError( + f"Failed to download bronze training_inputs file: {e}" + ) from e + + stream = getattr(response, "contents", None) + if stream is None: + raise ValueError("Databricks download returned no contents.") + return stream + # Note that for each unique PIPELINE, we'll need a new function, this is by nature of how unique pipelines # may have unique parameters and would have a unique name (i.e. the name field specified in w.jobs.list()). But any run of a given pipeline (even across institutions) can use the same function. # E.g. there is one PDP inference pipeline, so one PDP inference function here. diff --git a/src/webapp/databricks_test.py b/src/webapp/databricks_test.py index 3f84a52e..6bd9c627 100644 --- a/src/webapp/databricks_test.py +++ b/src/webapp/databricks_test.py @@ -308,6 +308,43 @@ def test_build_shared_inference_job_parameters_shape() -> None: ) +def test_download_bronze_training_inputs_rejects_path_traversal(ctrl) -> None: + with pytest.raises(ValueError, match="basename"): + ctrl.download_bronze_training_inputs_file("School", "../dataio.py") + with pytest.raises(ValueError, match="basename"): + ctrl.download_bronze_training_inputs_file("School", "foo/dataio.py") + with pytest.raises(ValueError, match="one of"): + ctrl.download_bronze_training_inputs_file("School", "secrets.py") + + +def test_download_bronze_training_inputs_file_builds_training_inputs_path( + monkeypatch: pytest.MonkeyPatch, ctrl +) -> None: + import src.webapp.databricks as db_mod + + monkeypatch.setitem( + db_mod.databricks_vars, "DATABRICKS_HOST_URL", "https://example.databricks.com" + ) + monkeypatch.setitem(db_mod.databricks_vars, "CATALOG_NAME", "dev_catalog") + monkeypatch.setitem( + db_mod.gcs_vars, "GCP_SERVICE_ACCOUNT_EMAIL", "sa@example.com" + ) + + mock_stream = MagicMock() + mock_response = MagicMock() + mock_response.contents = mock_stream + workspace = MagicMock() + workspace.files.download.return_value = mock_response + + with mock.patch.object(db_mod, "WorkspaceClient", return_value=workspace): + result = ctrl.download_bronze_training_inputs_file("Edvise School", "dataio.py") + + assert result is mock_stream + workspace.files.download.assert_called_once_with( + "/Volumes/dev_catalog/edvise_school_bronze/bronze_volume/training_inputs/dataio.py" + ) + + def test_run_validated_gcs_to_bronze_sync_calls_run_now_with_bundle_params( monkeypatch: pytest.MonkeyPatch, ) -> None: diff --git a/src/webapp/gcsutil.py b/src/webapp/gcsutil.py index 7c8f01e2..a289a1e1 100644 --- a/src/webapp/gcsutil.py +++ b/src/webapp/gcsutil.py @@ -396,8 +396,8 @@ def validate_file( file_name: Blob name under unvalidated/. allowed_schemas: List of schema/model names allowed. institution_id: Validation namespace: "edvise", "pdp", or "legacy". - institution_identifier: Optional institution ID (e.g. UUID). Used by - Edvise upload validation for caller context. + institution_identifier: For ES, institution name used to fetch bronze + ``training_inputs/dataio.py`` converters. Unused for PDP/Legacy. Returns: List of inferred schema names (e.g. ["STUDENT"]). diff --git a/src/webapp/routers/data.py b/src/webapp/routers/data.py index 32212d08..5d554eef 100644 --- a/src/webapp/routers/data.py +++ b/src/webapp/routers/data.py @@ -1613,6 +1613,7 @@ def _run_validation_and_upsert_file_record( current_user: BaseUser, storage_control: StorageControl, sess: Session, + institution_name: Optional[str] = None, ) -> Dict[str, Any]: """Run storage validate_file, then upsert file record and return response dict.""" try: @@ -1621,7 +1622,9 @@ def _run_validation_and_upsert_file_record( file_name, allowed_schemas, institution_id=schema_namespace, - institution_identifier=inst_id if schema_namespace == "edvise" else None, + institution_identifier=( + institution_name if schema_namespace == "edvise" else None + ), ) except HardValidationError as e: logging.debug("Inferred Schemas FAILED (hard) %s", e) @@ -1781,6 +1784,7 @@ def validation_helper( current_user, storage_control, sess, + institution_name=inst.name, ) if batch_id is not None: _load_batch_for_bronze_sync(sess, inst_id, batch_id) diff --git a/src/webapp/routers/data_test.py b/src/webapp/routers/data_test.py index 60e207f7..b867c4db 100644 --- a/src/webapp/routers/data_test.py +++ b/src/webapp/routers/data_test.py @@ -1464,7 +1464,7 @@ def test_validate_file_with_edvise_schema(edvise_client: TestClient) -> None: assert MOCK_STORAGE.validate_file.called call_kwargs = MOCK_STORAGE.validate_file.call_args.kwargs assert call_kwargs.get("institution_id") == "edvise" - assert call_kwargs.get("institution_identifier") == uuid_to_str(EDVISE_INST_UUID) + assert call_kwargs.get("institution_identifier") == "edvise_school" # Non-PDP bronze sync runs only when batch_id is provided at validation time. MOCK_DATABRICKS.run_validated_gcs_to_bronze_sync.assert_not_called() @@ -1856,7 +1856,7 @@ def test_validate_file_with_edvise_does_not_require_active_schema_doc( assert MOCK_STORAGE.validate_file.call_args.kwargs.get("institution_id") == "edvise" assert MOCK_STORAGE.validate_file.call_args.kwargs.get( "institution_identifier" - ) == uuid_to_str(EDVISE_INST_UUID) + ) == "edvise_school" def test_validation_helper_pdp_and_edvise_mutual_exclusivity( @@ -1995,7 +1995,7 @@ def capture_schema(*args, **kwargs): assert captured_arg_count == 3 assert captured_institution_id == "edvise" - assert captured_institution_identifier == uuid_to_str(EDVISE_INST_UUID) + assert captured_institution_identifier == "edvise_school" # Reset mock MOCK_STORAGE.validate_file.side_effect = None diff --git a/src/webapp/validation.py b/src/webapp/validation.py index 492b49a8..c715a4e6 100644 --- a/src/webapp/validation.py +++ b/src/webapp/validation.py @@ -1,17 +1,24 @@ """File validation functions for upload workflows. -PDP and Edvise uploads validate through Pandera schemas imported from the -``edvise`` package. Legacy uploads keep the API's any-format CSV read plus PII -guard. The old API-local JSON schema validation path has been removed. +PDP uploads validate through Pandera schemas imported from the ``edvise`` +package (optional school converters may be passed in). Edvise Schema (ES) +uploads fetch school ``training_inputs/dataio.py`` converters from the +institution bronze volume (when present), then validate via +``read_raw_es_*`` + ES Pandera schemas — parity with ES Databricks data-audit +jobs. Legacy uploads use any-format CSV read plus a PII column-name guard. +The old API-local JSON schema validation path has been removed. """ from __future__ import annotations import io +import importlib.util import os import re import logging +import sys import tempfile +import uuid from contextlib import contextmanager from functools import lru_cache from typing import ( @@ -22,6 +29,7 @@ Generator, List, Optional, + Tuple, Union, cast, ) @@ -29,12 +37,17 @@ import pandas as pd from pandera.errors import SchemaError, SchemaErrors -from edvise.dataio.read import read_raw_pdp_cohort_data, read_raw_pdp_course_data +from edvise.dataio.read import ( + read_raw_es_cohort_data, + read_raw_es_course_data, + read_raw_pdp_cohort_data, + read_raw_pdp_course_data, +) from edvise.utils.data_cleaning import handling_duplicates from . import validation_pdp_edvise as pdp_edvise -# Type for PDP converter functions (DataFrame -> DataFrame); used for cohort/course. +# Type for PDP/ES converter functions (DataFrame -> DataFrame); used for cohort/course. PDPConverterFunc = Optional[Callable[[pd.DataFrame], pd.DataFrame]] @@ -79,7 +92,7 @@ def validate_file_reader( filename: Path or file-like object for the CSV. allowed_schema: List of model names to validate against. institution_id: Validation namespace: "edvise", "pdp", or "legacy". - institution_identifier: Optional institution identifier (e.g. UUID) for display/context. + institution_identifier: For ES, institution name for bronze ``dataio`` lookup. pdp_cohort_converter_func: Optional cohort row transform before Pandera; default None. Batch PDP jobs may still apply school-specific cohort converters via ``dataio``. pdp_course_converter_func: Optional course converter; default duplicate handling only. @@ -225,6 +238,157 @@ def _model_list_from_models(models: Union[str, List[str], None]) -> List[str]: # Datetime formats to try for PDP course (same order as pdp_data_audit) PDP_COURSE_DTTM_FORMATS = ("ISO8601", "%Y%m%d.0", "%Y%m%d") +# Datetime formats for ES cohort/course (same order as es_data_audit) +ES_DTTM_FORMATS = ("ISO8601", "%Y%m%d.0") + + +def _reject_pii_columns(columns: Any) -> None: + """Raise HardValidationError if any column name looks like PII.""" + # Lazy import to avoid circular dependency: validation_error_formatter imports from this module. + from .validation_error_formatter import _is_pii_column + + pii_columns = [str(c) for c in columns if _is_pii_column(str(c))] + if pii_columns: + logger.warning("Upload rejected: PII columns detected: %s", pii_columns) + raise HardValidationError( + schema_errors=( + "Upload: file contains columns that may contain personally " + "identifiable information (PII). Please remove or de-identify " + "these columns before uploading." + ), + failure_cases=pii_columns, + ) + + +def _read_stream_to_bytes(stream: Any) -> bytes: + """Read a Databricks files download stream (or bytes-like) into bytes.""" + if isinstance(stream, (bytes, bytearray)): + return bytes(stream) + read = getattr(stream, "read", None) + if not callable(read): + raise ValueError("Download stream has no read() method.") + data = read() + if isinstance(data, str): + return data.encode("utf-8") + if not isinstance(data, (bytes, bytearray)): + raise ValueError("Download stream read() did not return bytes.") + return bytes(data) + + +def _import_dataio_module_isolated(inst_name: str, file_path: str) -> Any: + """ + Import a ``dataio.py`` file under a unique module name (no shared ``dataio`` cache). + + Does not mutate ``sys.path``. Registers the module under a unique name so + converters from different institutions never collide in ``sys.modules``. + """ + safe = re.sub(r"[^a-zA-Z0-9_]", "_", inst_name) or "unknown" + module_name = f"es_bronze_dataio_{safe}_{uuid.uuid4().hex}" + spec = importlib.util.spec_from_file_location(module_name, file_path) + if spec is None or spec.loader is None: + raise ImportError(f"Could not create import spec for {file_path}") + module = importlib.util.module_from_spec(spec) + # Unique name only — never reuse bare "dataio". + sys.modules[module_name] = module + try: + spec.loader.exec_module(module) + except Exception: + sys.modules.pop(module_name, None) + raise + return module + + +def load_es_converters_from_bronze( + inst_name: str, +) -> Tuple[PDPConverterFunc, PDPConverterFunc]: + """ + Download ``training_inputs/dataio.py`` from the institution bronze volume and + load ``converter_func_cohort`` / ``converter_func_course`` when present. + + Soft-falls back to ``(None, None)`` on missing file, download failure, import + failure, or missing attributes — parity with ES Databricks data-audit jobs. + Fresh fetch per call; no process-wide shared ``dataio`` module. + + Converter *runtime* errors during validation are not soft-fallbacked; they + surface as ``HardValidationError`` (fail closed for upload UX). + + Args: + inst_name: Institution name used to resolve the bronze volume path. + + Returns: + ``(cohort_converter | None, course_converter | None)``. + """ + # Lazy import: DatabricksControl pulls in a large dependency surface. + from .databricks import DatabricksControl + + tmp_path: Optional[str] = None + try: + stream = DatabricksControl().download_bronze_training_inputs_file( + inst_name, relative_path="dataio.py" + ) + raw = _read_stream_to_bytes(stream) + fd, tmp_path = tempfile.mkstemp(suffix="_dataio.py", prefix="es_bronze_") + try: + os.write(fd, raw) + finally: + os.close(fd) + + module = _import_dataio_module_isolated(inst_name, tmp_path) + except Exception as e: + logger.warning( + "ES bronze dataio unavailable for institution=%s; validating without " + "converters: %s", + inst_name, + e, + ) + return None, None + finally: + if tmp_path is not None: + try: + os.unlink(tmp_path) + except OSError: + pass + + cohort_converter: PDPConverterFunc = None + course_converter: PDPConverterFunc = None + try: + cohort_converter = module.converter_func_cohort + logger.info( + "Loaded custom ES cohort converter for institution=%s", inst_name + ) + except Exception as e: + logger.info( + "Running ES validation with default cohort converter for institution=%s", + inst_name, + ) + logger.warning("Failed to load custom ES cohort converter: %s", e) + try: + course_converter = module.converter_func_course + logger.info( + "Loaded custom ES course converter for institution=%s", inst_name + ) + except Exception as e: + logger.info( + "Running ES validation with default course converter for institution=%s", + inst_name, + ) + logger.warning("Failed to load custom ES course converter: %s", e) + + if cohort_converter is not None and not callable(cohort_converter): + logger.warning( + "converter_func_cohort for institution=%s is not callable; ignoring", + inst_name, + ) + cohort_converter = None + if course_converter is not None and not callable(course_converter): + logger.warning( + "converter_func_course for institution=%s is not callable; ignoring", + inst_name, + ) + course_converter = None + + return cohort_converter, course_converter + def _validate_pdp_converter_callables( pdp_cohort_converter_func: PDPConverterFunc, @@ -410,45 +574,124 @@ def _validate_edvise_with_repo_schema( enc: str, model_list: List[str], institution_id: str, + cohort_converter_func: PDPConverterFunc = None, + course_converter_func: PDPConverterFunc = None, ) -> Dict[str, Any]: - """Validate Edvise Schema uploads with upstream raw Edvise Pandera schemas.""" + """ + Validate Edvise Schema uploads via ``read_raw_es_*`` + raw Edvise Pandera schemas. + + Matches ES Databricks data-audit: optional school converters, then schema + validation with the same datetime-format retry order as ``es_data_audit``. + Converter runtime errors fail closed as ``HardValidationError``. + """ schema_class = pdp_edvise.get_edvise_schema_for_upload(institution_id, model_list) if schema_class is None: raise HardValidationError( schema_errors=f"Edvise repo schema expected; got models={model_list}", failure_cases=[], ) + model_set = {str(m).strip().upper() for m in model_list if m} with _path_for_edvise_read(filename, enc) as path: read_enc = "utf-8" if not isinstance(filename, (str, os.PathLike)) else enc try: - df = pd.read_csv(path, encoding=read_enc, dtype="string") + header_df = pd.read_csv(path, encoding=read_enc, nrows=0) except ( pd.errors.ParserError, pd.errors.EmptyDataError, UnicodeDecodeError, OSError, ) as e: - logger.exception("Edvise CSV read failed: %s", e) + logger.exception("Edvise CSV header read failed: %s", e) raise HardValidationError( schema_errors="Edvise upload: could not read CSV.", failure_cases=[str(e)], ) from e + _reject_pii_columns(header_df.columns) + + try: + df = _read_es_validated_dataframe( + path, + model_set, + schema_class, + cohort_converter_func, + course_converter_func, + ) + return { + "validation_status": "passed", + "schemas": model_list, + "missing_optional": [], + "unknown_extra_columns": [], + "normalized_df": df, + } + except (SchemaErrors, SchemaError) as e: + _convert_pdp_schema_errors_to_hard(e, model_set) + except HardValidationError: + raise + except Exception as e: + logger.exception( + "ES validation failed: model_set=%s, error=%s", model_set, e + ) + raise HardValidationError( + schema_errors=f"ES validation failed (model_set={model_set!r}): {e}", + failure_cases=[str(e)], + ) from e + + return {} # Unreachable: every path above returns or raises + - validated_df = pdp_edvise.validate_dataframe_with_edvise_schema( - df, - schema_class, - raw_to_canon={}, - canon_to_raw={}, - merged_specs={}, +def _read_es_validated_dataframe( + path: str, + model_set: set[str], + schema_class: type, + cohort_converter_func: PDPConverterFunc, + course_converter_func: PDPConverterFunc, +) -> pd.DataFrame: + """Read and validate ES cohort or course data; return validated DataFrame or raise.""" + if model_set == {"STUDENT"}: + last_error: Optional[Exception] = None + for fmt in ES_DTTM_FORMATS: + try: + return read_raw_es_cohort_data( + file_path=path, + schema=schema_class, + dttm_format=fmt, + converter_func=cohort_converter_func, + spark_session=None, + ) + except ValueError as e: + last_error = e + continue + raise HardValidationError( + schema_errors=( + "Failed to parse ES cohort data with all known datetime formats." + ), + failure_cases=[str(last_error)] if last_error else [], + ) + if model_set == {"COURSE"}: + last_error = None + for fmt in ES_DTTM_FORMATS: + try: + return read_raw_es_course_data( + file_path=path, + schema=schema_class, + dttm_format=fmt, + converter_func=course_converter_func, + spark_session=None, + ) + except ValueError as e: + last_error = e + continue + raise HardValidationError( + schema_errors=( + "Failed to parse ES course data with all known datetime formats." + ), + failure_cases=[str(last_error)] if last_error else [], + ) + raise HardValidationError( + schema_errors=f"ES single-model expected; got models={list(model_set)}", + failure_cases=[], ) - return { - "validation_status": "passed", - "schemas": model_list, - "missing_optional": [], - "unknown_extra_columns": [], - "normalized_df": validated_df, - } def _validate_pdp_with_edvise_read( @@ -529,16 +772,16 @@ def _validate_pdp_with_edvise_read( # --------------------------------------------------------------------------- # -def _validate_legacy_any_format( +def _validate_any_format_csv( filename: Src, enc: str, models: Union[str, List[str], None], ) -> Dict[str, Any]: """ - Legacy institutions: accept any CSV format (encoding check only, no schema). + Accept any CSV format (encoding/parse + PII column-name guard; no Pandera). - Reads the file as CSV with no column or type checks; returns the DataFrame - as-is as normalized_df so it can be written to validated/. + Used for Legacy uploads. Reads the file as CSV with no column or type checks; + returns the DataFrame as-is as normalized_df so it can be written to validated/. Args: filename: Path or file-like object for the CSV. @@ -573,32 +816,18 @@ def _validate_legacy_any_format( UnicodeDecodeError, OSError, ) as e: - logger.exception("Legacy CSV read failed: %s", e) + logger.exception("Any-format CSV read failed: %s", e) raise HardValidationError( - schema_errors="Legacy upload: could not read CSV.", + schema_errors="Upload: could not read CSV.", failure_cases=[str(e)], ) from e if df is None or not isinstance(df, pd.DataFrame): df = pd.DataFrame() - # PII check: reject legacy uploads that contain columns indicating PII (before moving to raw/validated). + # PII check: reject uploads with columns indicating PII (before raw/validated). # Run whenever there are columns (including header-only CSVs: df.empty is True for 0 rows). if len(df.columns) > 0: - # Lazy import to avoid circular dependency: validation_error_formatter imports from this module. - from .validation_error_formatter import _is_pii_column - - pii_columns = [str(c) for c in df.columns if _is_pii_column(str(c))] - if pii_columns: - logger.warning( - "Legacy upload rejected: PII columns detected: %s", pii_columns - ) - raise HardValidationError( - schema_errors=( - "Legacy upload: file contains columns that may contain personally identifiable information (PII). " - "Please remove or de-identify these columns before uploading." - ), - failure_cases=pii_columns, - ) + _reject_pii_columns(df.columns) return { "validation_status": "passed", @@ -609,6 +838,10 @@ def _validate_legacy_any_format( } +# Back-compat alias for callers/tests that still use the legacy name. +_validate_legacy_any_format = _validate_any_format_csv + + def validate_dataset( filename: Src, models: Union[str, List[str], None] = None, @@ -620,16 +853,19 @@ def validate_dataset( """ Validate a dataset using the active institution upload workflow. - Detects encoding, then routes to legacy any-format handling or PDP/Edvise - repo Pandera validation for supported single-model STUDENT/COURSE uploads. - Other PDP/Edvise model sets are rejected explicitly; the API-local JSON - schema validation fallback has been removed. + Detects encoding, then routes to Legacy any-format handling, ES + ``read_raw_es_*`` + Pandera (with optional bronze ``dataio`` converters), or + PDP repo Pandera validation for supported single-model STUDENT/COURSE uploads. + Other model sets are rejected explicitly; the API-local JSON schema + validation fallback has been removed. Args: filename: CSV path or file-like object. models: Model name(s) to validate. - institution_id: Validation namespace, or ``"legacy"`` for any-format validation. - institution_identifier: Optional UUID string for caller context (e.g. Edvise). + institution_id: Validation namespace (``"pdp"``, ``"edvise"``, or ``"legacy"``). + ``"legacy"`` skips Pandera; ``"edvise"`` and ``"pdp"`` use repo schemas. + institution_identifier: For ES, the institution name used to resolve the + bronze volume ``training_inputs/dataio.py`` path. Unused for PDP/Legacy. pdp_cohort_converter_func: Optional cohort transform before Pandera; default ``None``. Batch PDP jobs may still apply school-specific cohort converters via ``dataio``. pdp_course_converter_func: Optional course converter before default duplicate handling. @@ -648,18 +884,48 @@ def validate_dataset( raise HardValidationError(schema_errors="decode_error", failure_cases=[str(ex)]) _reset_to_start_if_possible(filename) + # Legacy: parse CSV + PII guard only (any format). if institution_id == "legacy": - return _validate_legacy_any_format(filename, enc, models) + return _validate_any_format_csv(filename, enc, models) model_list = _model_list_from_models(models) - schema_class = pdp_edvise.get_edvise_schema_for_upload(institution_id, model_list) - if schema_class is not None and institution_id == "edvise": + + # ES: bronze dataio converters (soft-fallback if missing) + read_raw_es_* + Pandera. + if institution_id == "edvise": + schema_class = pdp_edvise.get_edvise_schema_for_upload( + institution_id, model_list + ) + if schema_class is None: + supported = "STUDENT and COURSE single-model uploads" + requested = ", ".join(model_list) if model_list else "none" + raise HardValidationError( + schema_errors=( + f"{institution_id} upload validation only supports {supported} through " + f"the edvise repo. Requested model(s): {requested}." + ), + failure_cases=[], + ) + cohort_converter: PDPConverterFunc = None + course_converter: PDPConverterFunc = None + if institution_identifier: + cohort_converter, course_converter = load_es_converters_from_bronze( + institution_identifier + ) + else: + logger.warning( + "ES validation without institution_identifier; validating without " + "bronze dataio converters" + ) return _validate_edvise_with_repo_schema( filename, enc, model_list, institution_id, + cohort_converter_func=cohort_converter, + course_converter_func=course_converter, ) + + schema_class = pdp_edvise.get_edvise_schema_for_upload(institution_id, model_list) if schema_class is not None: return _validate_pdp_with_edvise_read( filename, diff --git a/src/webapp/validation_es_test.py b/src/webapp/validation_es_test.py new file mode 100644 index 00000000..ede45ffa --- /dev/null +++ b/src/webapp/validation_es_test.py @@ -0,0 +1,288 @@ +"""Tests for ES upload validation: bronze dataio converters + read_raw_es_* + Pandera.""" + +from __future__ import annotations + +import io +import sys +from pathlib import Path +from typing import Any, Callable +from unittest.mock import patch + +import pandas as pd +import pytest +from pandera.errors import SchemaError + +from src.webapp.validation import ( + HardValidationError, + _import_dataio_module_isolated, + load_es_converters_from_bronze, + validate_file_reader, +) + + +def _dataio_source(cohort_marker: str, course_marker: str) -> bytes: + return f''' +import pandas as pd + +SCHOOL_MARKER = "{cohort_marker}" + +def converter_func_cohort(df: pd.DataFrame) -> pd.DataFrame: + out = df.copy() + if "student_guid" in out.columns and "learner_id" not in out.columns: + out = out.rename(columns={{"student_guid": "learner_id"}}) + out.attrs["school_marker"] = "{cohort_marker}" + return out + +def converter_func_course(df: pd.DataFrame) -> pd.DataFrame: + out = df.copy() + out.attrs["school_marker"] = "{course_marker}" + return out +'''.encode( + "utf-8" + ) + + +def test_load_es_converters_applies_cohort_rename(tmp_path: Path) -> None: + """Converter from bronze dataio.py is loaded and renames columns like the ES job.""" + stream = io.BytesIO(_dataio_source("school_a", "school_a_course")) + + with patch( + "src.webapp.databricks.DatabricksControl.download_bronze_training_inputs_file", + return_value=stream, + ): + cohort_fn, course_fn = load_es_converters_from_bronze("school_a") + + assert callable(cohort_fn) + assert callable(course_fn) + df = pd.DataFrame({"student_guid": ["s1"], "entry_year": ["2024"]}) + converted = cohort_fn(df) + assert "learner_id" in converted.columns + assert "student_guid" not in converted.columns + assert converted.attrs["school_marker"] == "school_a" + + +def test_load_es_converters_missing_dataio_soft_fallback() -> None: + """Missing bronze dataio.py soft-falls back to no converters (job parity).""" + with patch( + "src.webapp.databricks.DatabricksControl.download_bronze_training_inputs_file", + side_effect=ValueError("Failed to download bronze training_inputs file: missing"), + ): + cohort_fn, course_fn = load_es_converters_from_bronze("school_missing") + + assert cohort_fn is None + assert course_fn is None + + +def test_es_converter_import_isolation_across_institutions() -> None: + """Two institutions with different dataio.py never share converters or bare dataio.""" + + def download_side_effect(inst_name: str, relative_path: str = "dataio.py") -> Any: + assert relative_path == "dataio.py" + if inst_name == "inst_alpha": + return io.BytesIO(_dataio_source("inst_alpha", "inst_alpha_c")) + if inst_name == "inst_beta": + return io.BytesIO(_dataio_source("inst_beta", "inst_beta_c")) + raise AssertionError(f"unexpected institution {inst_name}") + + with patch( + "src.webapp.databricks.DatabricksControl.download_bronze_training_inputs_file", + side_effect=download_side_effect, + ): + cohort_a, _ = load_es_converters_from_bronze("inst_alpha") + cohort_b, _ = load_es_converters_from_bronze("inst_beta") + + assert "dataio" not in sys.modules + assert callable(cohort_a) and callable(cohort_b) + assert cohort_a is not cohort_b + + df = pd.DataFrame({"student_guid": ["x"]}) + assert cohort_a(df).attrs["school_marker"] == "inst_alpha" + assert cohort_b(df).attrs["school_marker"] == "inst_beta" + + bronze_modules = [ + name for name in sys.modules if name.startswith("es_bronze_dataio_") + ] + assert len(bronze_modules) >= 2 + + +def test_import_dataio_module_isolated_unique_names(tmp_path: Path) -> None: + """spec_from_file_location uses unique module names per institution/request.""" + path = tmp_path / "dataio.py" + path.write_text("MARKER = 'ok'\n", encoding="utf-8") + mod1 = _import_dataio_module_isolated("school_one", str(path)) + mod2 = _import_dataio_module_isolated("school_two", str(path)) + assert mod1 is not mod2 + assert mod1.__name__ != mod2.__name__ + assert mod1.__name__.startswith("es_bronze_dataio_school_one_") + assert mod2.__name__.startswith("es_bronze_dataio_school_two_") + assert "dataio" not in sys.modules + + +def test_es_with_converter_passed_to_read_raw_es(tmp_path: Path) -> None: + """ES validate_file_reader passes the loaded cohort converter into read_raw_es_*.""" + csv_path = tmp_path / "student.csv" + csv_path.write_text("student_guid,entry_year\ns1,2024\n", encoding="utf-8") + + def cohort_converter(df: pd.DataFrame) -> pd.DataFrame: + return df.rename(columns={"student_guid": "learner_id"}) + + captured: dict[str, Any] = {} + + def fake_read(**kwargs: Any) -> pd.DataFrame: + captured["converter_func"] = kwargs.get("converter_func") + return pd.DataFrame({"learner_id": ["s1"]}) + + with ( + patch( + "src.webapp.validation.load_es_converters_from_bronze", + return_value=(cohort_converter, None), + ), + patch( + "src.webapp.validation.read_raw_es_cohort_data", + side_effect=fake_read, + ), + ): + result = validate_file_reader( + str(csv_path), + ["STUDENT"], + institution_id="edvise", + institution_identifier="edvise_school", + ) + + assert result["validation_status"] == "passed" + assert captured["converter_func"] is cohort_converter + + +def test_es_missing_dataio_validates_without_converter(tmp_path: Path) -> None: + """Missing dataio still validates via read_raw_es_* with converter_func=None.""" + csv_path = tmp_path / "student.csv" + csv_path.write_text("learner_id,entry_year\ns1,2024\n", encoding="utf-8") + captured: dict[str, Any] = {} + + def fake_read(**kwargs: Any) -> pd.DataFrame: + captured["converter_func"] = kwargs.get("converter_func") + return pd.DataFrame({"learner_id": ["s1"]}) + + with ( + patch( + "src.webapp.validation.load_es_converters_from_bronze", + return_value=(None, None), + ) as mock_load, + patch( + "src.webapp.validation.read_raw_es_cohort_data", + side_effect=fake_read, + ), + ): + result = validate_file_reader( + str(csv_path), + ["STUDENT"], + institution_id="edvise", + institution_identifier="edvise_school", + ) + + mock_load.assert_called_once_with("edvise_school") + assert result["validation_status"] == "passed" + assert captured["converter_func"] is None + + +def test_es_pandera_failure_returns_hard_validation_error(tmp_path: Path) -> None: + """Pandera SchemaErrors from read_raw_es_* surface as HardValidationError.""" + csv_path = tmp_path / "student.csv" + csv_path.write_text("learner_id\ns1\n", encoding="utf-8") + + # SchemaError requires schema/data kwargs; build a bare instance for the raise path. + pandera_err = SchemaError.__new__(SchemaError) + Exception.__init__(pandera_err, "missing required columns") + + with ( + patch( + "src.webapp.validation.load_es_converters_from_bronze", + return_value=(None, None), + ), + patch( + "src.webapp.validation.read_raw_es_cohort_data", + side_effect=pandera_err, + ), + patch( + "src.webapp.validation.pdp_edvise._convert_schema_errors_to_hard_validation_error", + return_value=HardValidationError( + schema_errors="ES pandera failed", + failure_cases=[{"column": "entry_year", "check": "column_in_dataframe"}], + ), + ), + ): + with pytest.raises(HardValidationError) as exc_info: + validate_file_reader( + str(csv_path), + ["STUDENT"], + institution_id="edvise", + institution_identifier="edvise_school", + ) + + assert "ES pandera failed" in str(exc_info.value.schema_errors) + + +def test_es_converter_runtime_error_fails_closed(tmp_path: Path) -> None: + """Converter exceptions during validation become HardValidationError (fail closed).""" + csv_path = tmp_path / "student.csv" + csv_path.write_text("learner_id\ns1\n", encoding="utf-8") + + def bad_converter(df: pd.DataFrame) -> pd.DataFrame: + raise RuntimeError("converter blew up") + + def fake_read(**kwargs: Any) -> pd.DataFrame: + converter: Callable[[pd.DataFrame], pd.DataFrame] | None = kwargs.get( + "converter_func" + ) + assert converter is not None + return converter(pd.DataFrame({"learner_id": ["s1"]})) + + with ( + patch( + "src.webapp.validation.load_es_converters_from_bronze", + return_value=(bad_converter, None), + ), + patch( + "src.webapp.validation.read_raw_es_cohort_data", + side_effect=fake_read, + ), + ): + with pytest.raises(HardValidationError) as exc_info: + validate_file_reader( + str(csv_path), + ["STUDENT"], + institution_id="edvise", + institution_identifier="edvise_school", + ) + + assert "converter blew up" in str(exc_info.value) + + +def test_pdp_routing_unchanged_does_not_load_es_converters(tmp_path: Path) -> None: + """PDP uploads do not fetch bronze ES dataio converters.""" + csv_path = tmp_path / "cohort.csv" + pd.DataFrame({"x": [1]}).to_csv(csv_path, index=False) + + with ( + patch( + "src.webapp.validation.load_es_converters_from_bronze", + ) as mock_load, + patch( + "src.webapp.validation._validate_pdp_with_edvise_read", + return_value={ + "validation_status": "passed", + "schemas": ["STUDENT"], + "missing_optional": [], + "unknown_extra_columns": [], + "normalized_df": pd.DataFrame({"student_id": ["s1"]}), + }, + ), + ): + result = validate_file_reader( + str(csv_path), + ["STUDENT"], + institution_id="pdp", + ) + + assert result["validation_status"] == "passed" + mock_load.assert_not_called() diff --git a/src/webapp/validation_pdp_edvise.py b/src/webapp/validation_pdp_edvise.py index 743eac14..ad79c49d 100644 --- a/src/webapp/validation_pdp_edvise.py +++ b/src/webapp/validation_pdp_edvise.py @@ -1,9 +1,9 @@ """Pandera schemas re-exported from edvise for upload validation. -Imports raw PDP and Edvise schema classes so uploads use the same column and type -rules as edvise pipeline audits. Cohort row transforms run in ``validation.py`` -(optional converter) and can differ from batch ``dataio`` hooks; this module only -supplies schema classes and helpers. +Imports raw PDP and Edvise schema classes. PDP and ES uploads use these at +upload time (ES after optional bronze ``dataio`` converters). Legacy uploads +skip Pandera (any-format CSV + PII guard). This module supplies schema classes +and helpers for the PDP/ES validation paths. Requires the ``edvise`` package (see pyproject.toml). """ @@ -34,7 +34,8 @@ def _get_hard_validation_error_class() -> type: return HardValidationError -# Institution namespaces that use edvise repo schemas for single-model uploads. +# Institution namespaces with edvise repo schema classes available. +# Upload-time Pandera runs for both "pdp" and "edvise" single-model STUDENT/COURSE. PDP_EDVISE_NAMESPACES = frozenset({"pdp", "edvise"}) @@ -84,9 +85,8 @@ def get_edvise_schema_for_upload( """ Return the edvise repo schema class for this upload, or None. - Use this as the single check: when not None, run that schema. PDP and - Edvise single-model STUDENT/COURSE uploads use repo validation (edvise - package required). + Use this as the single check: when not None, a repo schema class exists. + PDP and ES single-model STUDENT/COURSE uploads use these at upload time. Args: institution_id: Schema namespace (e.g. "pdp", "edvise", or "legacy"). diff --git a/src/webapp/validation_pdp_read_path_test.py b/src/webapp/validation_pdp_read_path_test.py index cc983dff..e5a32577 100644 --- a/src/webapp/validation_pdp_read_path_test.py +++ b/src/webapp/validation_pdp_read_path_test.py @@ -92,14 +92,18 @@ def test_validate_file_reader_pdp_course_calls_edvise_read_path(tmp_path: Path) assert mock_pdp.call_args[0][2] == ["COURSE"] -def test_validate_file_reader_edvise_student_calls_repo_schema_path( +def test_validate_file_reader_edvise_student_uses_repo_schema_path( tmp_path: Path, ) -> None: - """When institution_id is edvise and allowed_schema is [STUDENT], Edvise repo schema path is used.""" + """When institution_id is edvise, uploads use read_raw_es_* + Pandera.""" csv_path = tmp_path / "edvise_student.csv" pd.DataFrame({"learner_id": ["s1"]}).to_csv(csv_path, index=False) with ( + patch( + "src.webapp.validation.load_es_converters_from_bronze", + return_value=(None, None), + ) as mock_load, patch( "src.webapp.validation._validate_edvise_with_repo_schema", return_value={ @@ -109,76 +113,71 @@ def test_validate_file_reader_edvise_student_calls_repo_schema_path( "unknown_extra_columns": [], "normalized_df": pd.DataFrame({"learner_id": ["s1"]}), }, - ) as mock_edvise_schema, + ) as mock_edvise, + patch( + "src.webapp.validation._validate_any_format_csv", + ) as mock_any_format, ): result = validate_file_reader( str(csv_path), ["STUDENT"], institution_id="edvise", + institution_identifier="edvise_school", ) assert result["validation_status"] == "passed" assert result["schemas"] == ["STUDENT"] - mock_edvise_schema.assert_called_once() - call_args = mock_edvise_schema.call_args[0] - assert call_args[2] == ["STUDENT"] - assert call_args[3] == "edvise" + mock_load.assert_called_once_with("edvise_school") + mock_edvise.assert_called_once() + assert mock_edvise.call_args.args[2] == ["STUDENT"] + mock_any_format.assert_not_called() -def test_validate_file_reader_edvise_routes_before_schema_merge( +def test_validate_file_reader_edvise_calls_repo_schema_path( tmp_path: Path, ) -> None: - """Edvise repo validation should not depend on populated JSON schema docs.""" + """ES upload validation must invoke repo Pandera schema validation.""" csv_path = tmp_path / "edvise_student.csv" pd.DataFrame({"learner_id": ["s1"]}).to_csv(csv_path, index=False) - with patch( - "src.webapp.validation._validate_edvise_with_repo_schema", - return_value={ - "validation_status": "passed", - "schemas": ["STUDENT"], - "missing_optional": [], - "unknown_extra_columns": [], - "normalized_df": pd.DataFrame({"learner_id": ["s1"]}), - }, - ) as mock_edvise_schema: + with ( + patch( + "src.webapp.validation.load_es_converters_from_bronze", + return_value=(None, None), + ), + patch( + "src.webapp.validation._validate_edvise_with_repo_schema", + return_value={ + "validation_status": "passed", + "schemas": ["STUDENT"], + "missing_optional": [], + "unknown_extra_columns": [], + "normalized_df": pd.DataFrame({"learner_id": ["s1"]}), + }, + ) as mock_edvise_schema, + ): result = validate_file_reader( str(csv_path), ["STUDENT"], institution_id="edvise", + institution_identifier="edvise_school", ) assert result["validation_status"] == "passed" mock_edvise_schema.assert_called_once() -def test_validate_edvise_with_repo_schema_preserves_string_values( +def test_validate_edvise_with_repo_schema_uses_read_raw_es( tmp_path: Path, ) -> None: - """Edvise CSV loading preserves leading zeros before Pandera coercion.""" + """Edvise validation path calls read_raw_es_cohort_data with the schema.""" csv_path = tmp_path / "edvise_student.csv" csv_path.write_text("learner_id,entry_year\n00123,2024\n") - schema_class = object() - captured_df: pd.DataFrame | None = None - - def capture_validation_df( - df: pd.DataFrame, - *args: object, - **kwargs: object, - ) -> pd.DataFrame: - nonlocal captured_df - captured_df = df - return df + expected = pd.DataFrame({"learner_id": ["00123"], "entry_year": ["2024"]}) - with ( - patch( - "src.webapp.validation.pdp_edvise.get_edvise_schema_for_upload", - return_value=schema_class, - ), - patch( - "src.webapp.validation.pdp_edvise.validate_dataframe_with_edvise_schema", - side_effect=capture_validation_df, - ) as mock_validate, - ): + with patch( + "src.webapp.validation.read_raw_es_cohort_data", + return_value=expected, + ) as mock_read: result = _validate_edvise_with_repo_schema( str(csv_path), enc="utf-8", @@ -187,11 +186,10 @@ def capture_validation_df( ) assert result["validation_status"] == "passed" - assert captured_df is not None - assert captured_df.loc[0, "learner_id"] == "00123" - assert captured_df["learner_id"].dtype.name == "string" - mock_validate.assert_called_once() - assert mock_validate.call_args[0][1] is schema_class + assert result["normalized_df"] is expected + mock_read.assert_called_once() + assert mock_read.call_args.kwargs["file_path"] == str(csv_path) + assert mock_read.call_args.kwargs["converter_func"] is None # --------------------------------------------------------------------------- # diff --git a/src/webapp/validation_test.py b/src/webapp/validation_test.py index 86aba555..3448d7f8 100644 --- a/src/webapp/validation_test.py +++ b/src/webapp/validation_test.py @@ -206,33 +206,40 @@ def test_validate_file_reader_pdp_student_routes_to_repo_validation( assert mock_validate.call_args.args[3] == "pdp" -def test_validate_file_reader_edvise_course_routes_to_repo_validation( +def test_validate_file_reader_edvise_course_uses_repo_schema_path( tmp_csv_file: str, ) -> None: - """Edvise COURSE uploads use repo-backed validation, not JSON schema docs.""" - expected_df = pd.DataFrame({"course_id": ["c1"]}) - - with patch( - "src.webapp.validation._validate_edvise_with_repo_schema", - return_value={ - "validation_status": "passed", - "schemas": ["COURSE"], - "missing_optional": [], - "unknown_extra_columns": [], - "normalized_df": expected_df, - }, - ) as mock_validate: + """Edvise COURSE uploads use read_raw_es_* + Pandera (not any-format).""" + with ( + patch( + "src.webapp.validation.load_es_converters_from_bronze", + return_value=(None, None), + ), + patch( + "src.webapp.validation._validate_edvise_with_repo_schema", + return_value={ + "validation_status": "passed", + "schemas": ["COURSE"], + "missing_optional": [], + "unknown_extra_columns": [], + "normalized_df": pd.DataFrame({"course_id": ["c1"]}), + }, + ) as mock_validate, + patch( + "src.webapp.validation._validate_any_format_csv", + ) as mock_any_format, + ): result = validate_file_reader( tmp_csv_file, ["COURSE"], institution_id="edvise", + institution_identifier="edvise_school", ) assert result["validation_status"] == "passed" - assert result["normalized_df"] is expected_df mock_validate.assert_called_once() assert mock_validate.call_args.args[2] == ["COURSE"] - assert mock_validate.call_args.args[3] == "edvise" + mock_any_format.assert_not_called() def test_validate_file_reader_pdp_rejects_unsupported_model_set( @@ -250,16 +257,44 @@ def test_validate_file_reader_pdp_rejects_unsupported_model_set( assert "SEMESTER" in str(exc_info.value) -def test_validate_file_reader_edvise_rejects_multi_model_upload( - tmp_csv_file: str, +def test_validate_file_reader_edvise_rejects_schema_invalid_csv( + tmp_path: Path, ) -> None: - """Edvise multi-model uploads are rejected instead of using old JSON validation.""" - with pytest.raises(HardValidationError) as exc_info: - validate_file_reader( - tmp_csv_file, - ["STUDENT", "COURSE"], - institution_id="edvise", - ) + """ES uploads fail Pandera when columns do not match the raw Edvise schema.""" + csv_path = tmp_path / "messy_student.csv" + csv_path.write_text("weird_col,another\nx,y\n", encoding="utf-8") - assert "edvise repo" in str(exc_info.value) - assert "STUDENT, COURSE" in str(exc_info.value) + with patch( + "src.webapp.validation.load_es_converters_from_bronze", + return_value=(None, None), + ): + with pytest.raises(HardValidationError) as exc_info: + validate_file_reader( + str(csv_path), + ["STUDENT"], + institution_id="edvise", + institution_identifier="edvise_school", + ) + + assert exc_info.value.schema_errors is not None or exc_info.value.failure_cases + + +def test_validate_file_reader_edvise_rejects_pii_columns(tmp_path: Path) -> None: + """ES path still rejects PII-looking column names before schema validation.""" + csv_path = tmp_path / "with_pii.csv" + csv_path.write_text("learner_id,email\n1,a@b.com\n", encoding="utf-8") + + with patch( + "src.webapp.validation.load_es_converters_from_bronze", + return_value=(None, None), + ): + with pytest.raises(HardValidationError) as exc_info: + validate_file_reader( + str(csv_path), + ["STUDENT"], + institution_id="edvise", + institution_identifier="edvise_school", + ) + + assert "PII" in (exc_info.value.schema_errors or "") + assert "email" in (exc_info.value.failure_cases or []) From 655fe70f839732786ef23275a9c7c0a3a4a4d226 Mon Sep 17 00:00:00 2001 From: Vishakh Pillai Date: Mon, 20 Jul 2026 12:49:22 -0400 Subject: [PATCH 02/10] fix: style --- src/webapp/databricks_test.py | 4 +--- src/webapp/routers/data_test.py | 7 ++++--- src/webapp/validation.py | 8 ++------ src/webapp/validation_es_test.py | 12 +++++++----- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/webapp/databricks_test.py b/src/webapp/databricks_test.py index 6bd9c627..14c38e9c 100644 --- a/src/webapp/databricks_test.py +++ b/src/webapp/databricks_test.py @@ -326,9 +326,7 @@ def test_download_bronze_training_inputs_file_builds_training_inputs_path( db_mod.databricks_vars, "DATABRICKS_HOST_URL", "https://example.databricks.com" ) monkeypatch.setitem(db_mod.databricks_vars, "CATALOG_NAME", "dev_catalog") - monkeypatch.setitem( - db_mod.gcs_vars, "GCP_SERVICE_ACCOUNT_EMAIL", "sa@example.com" - ) + monkeypatch.setitem(db_mod.gcs_vars, "GCP_SERVICE_ACCOUNT_EMAIL", "sa@example.com") mock_stream = MagicMock() mock_response = MagicMock() diff --git a/src/webapp/routers/data_test.py b/src/webapp/routers/data_test.py index b867c4db..92e4fb46 100644 --- a/src/webapp/routers/data_test.py +++ b/src/webapp/routers/data_test.py @@ -1854,9 +1854,10 @@ def test_validate_file_with_edvise_does_not_require_active_schema_doc( assert response.status_code == 200 assert MOCK_STORAGE.validate_file.call_args.kwargs.get("institution_id") == "edvise" - assert MOCK_STORAGE.validate_file.call_args.kwargs.get( - "institution_identifier" - ) == "edvise_school" + assert ( + MOCK_STORAGE.validate_file.call_args.kwargs.get("institution_identifier") + == "edvise_school" + ) def test_validation_helper_pdp_and_edvise_mutual_exclusivity( diff --git a/src/webapp/validation.py b/src/webapp/validation.py index c715a4e6..e7c63841 100644 --- a/src/webapp/validation.py +++ b/src/webapp/validation.py @@ -353,9 +353,7 @@ def load_es_converters_from_bronze( course_converter: PDPConverterFunc = None try: cohort_converter = module.converter_func_cohort - logger.info( - "Loaded custom ES cohort converter for institution=%s", inst_name - ) + logger.info("Loaded custom ES cohort converter for institution=%s", inst_name) except Exception as e: logger.info( "Running ES validation with default cohort converter for institution=%s", @@ -364,9 +362,7 @@ def load_es_converters_from_bronze( logger.warning("Failed to load custom ES cohort converter: %s", e) try: course_converter = module.converter_func_course - logger.info( - "Loaded custom ES course converter for institution=%s", inst_name - ) + logger.info("Loaded custom ES course converter for institution=%s", inst_name) except Exception as e: logger.info( "Running ES validation with default course converter for institution=%s", diff --git a/src/webapp/validation_es_test.py b/src/webapp/validation_es_test.py index ede45ffa..3c453061 100644 --- a/src/webapp/validation_es_test.py +++ b/src/webapp/validation_es_test.py @@ -37,9 +37,7 @@ def converter_func_course(df: pd.DataFrame) -> pd.DataFrame: out = df.copy() out.attrs["school_marker"] = "{course_marker}" return out -'''.encode( - "utf-8" - ) +'''.encode("utf-8") def test_load_es_converters_applies_cohort_rename(tmp_path: Path) -> None: @@ -65,7 +63,9 @@ def test_load_es_converters_missing_dataio_soft_fallback() -> None: """Missing bronze dataio.py soft-falls back to no converters (job parity).""" with patch( "src.webapp.databricks.DatabricksControl.download_bronze_training_inputs_file", - side_effect=ValueError("Failed to download bronze training_inputs file: missing"), + side_effect=ValueError( + "Failed to download bronze training_inputs file: missing" + ), ): cohort_fn, course_fn = load_es_converters_from_bronze("school_missing") @@ -207,7 +207,9 @@ def test_es_pandera_failure_returns_hard_validation_error(tmp_path: Path) -> Non "src.webapp.validation.pdp_edvise._convert_schema_errors_to_hard_validation_error", return_value=HardValidationError( schema_errors="ES pandera failed", - failure_cases=[{"column": "entry_year", "check": "column_in_dataframe"}], + failure_cases=[ + {"column": "entry_year", "check": "column_in_dataframe"} + ], ), ), ): From 3c3e250d178c444f26eb7f905d0dd131fc9783bd Mon Sep 17 00:00:00 2001 From: Noreen Mayat Date: Mon, 20 Jul 2026 12:49:42 -0700 Subject: [PATCH 03/10] feat: creating archive endpoint creating endpoint to archive models from front end. --- src/webapp/database.py | 1 + src/webapp/routers/models.py | 41 +++++++++++++++++++++++++++++++ src/webapp/routers/models_test.py | 17 +++++++++++++ 3 files changed, 59 insertions(+) diff --git a/src/webapp/database.py b/src/webapp/database.py index e5e1d541..f5d554a3 100644 --- a/src/webapp/database.py +++ b/src/webapp/database.py @@ -646,6 +646,7 @@ class ModelTable(Base): deleted: Mapped[bool] = mapped_column(nullable=True) # If true, the model has been approved and is ready for use. valid: Mapped[bool] = mapped_column(nullable=True) + archived: Mapped[int] = mapped_column(Integer, default=0) # The time the deletion request was set. deleted_at: Mapped[datetime.datetime] = mapped_column( DateTime(timezone=True), nullable=True diff --git a/src/webapp/routers/models.py b/src/webapp/routers/models.py index c94763a6..e7393e21 100644 --- a/src/webapp/routers/models.py +++ b/src/webapp/routers/models.py @@ -466,6 +466,47 @@ def delete_model( } +@router.patch("/{inst_id}/models/{model_name}/archive") +def archive_model( + inst_id: str, + model_name: str, + current_user: Annotated[BaseUser, Depends(get_current_active_user)], + sql_session: Annotated[Session, Depends(get_session)], +) -> Any: + """Archive a model by setting ``archived`` from 0 to 1.""" + transformed_model_name = str(decode_url_piece(model_name)).strip() + has_access_to_inst_or_err(inst_id, current_user) + + local_session.set(sql_session) + sess = local_session.get() + + model = sess.execute( + select(ModelTable).where( + ModelTable.name == transformed_model_name, + ModelTable.inst_id == str_to_uuid(inst_id), + ) + ).scalar_one_or_none() + if model is None: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="Model not found." + ) + if model.archived: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail="Model is already archived.", + ) + + model.archived = 1 + sess.commit() + + return { + "inst_id": inst_id, + "model_name": transformed_model_name, + "archived": 1, + "status": "Model archived", + } + + @router.get("/{inst_id}/models/{model_name}/runs", response_model=list[RunInfo]) def read_inst_model_outputs( inst_id: str, diff --git a/src/webapp/routers/models_test.py b/src/webapp/routers/models_test.py index 19467e9b..e8f09278 100644 --- a/src/webapp/routers/models_test.py +++ b/src/webapp/routers/models_test.py @@ -283,6 +283,23 @@ def test_read_inst_model(client: TestClient) -> None: assert same_model_orderless(response_model, expected_model) +def test_archive_model(client: TestClient, session: sqlalchemy.orm.Session) -> None: + """Test PATCH /institutions/{inst_id}/models/{model_name}/archive.""" + base = "/institutions/" + uuid_to_str(USER_VALID_INST_UUID) + "/models/" + assert client.patch(base + "missing_model/archive").status_code == 404 + + response = client.patch(base + "sample_model_for_school_1/archive") + assert response.status_code == 200 + assert response.json() == { + "inst_id": uuid_to_str(USER_VALID_INST_UUID), + "model_name": "sample_model_for_school_1", + "archived": 1, + "status": "Model archived", + } + assert session.get(ModelTable, SAMPLE_UUID).archived == 1 + assert client.patch(base + "sample_model_for_school_1/archive").status_code == 409 + + def test_read_inst_model_outputs(client: TestClient) -> None: """Test GET /institutions/345/models/10/output.""" MOCK_STORAGE.list_blobs_in_folder.return_value = [] From c734f7c160bca7be2f9fb5c78a8be5d4a0e37bde Mon Sep 17 00:00:00 2001 From: Noreen Mayat Date: Mon, 20 Jul 2026 12:53:43 -0700 Subject: [PATCH 04/10] fix: type check error --- src/webapp/routers/models_test.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/webapp/routers/models_test.py b/src/webapp/routers/models_test.py index e8f09278..7911fd76 100644 --- a/src/webapp/routers/models_test.py +++ b/src/webapp/routers/models_test.py @@ -296,7 +296,9 @@ def test_archive_model(client: TestClient, session: sqlalchemy.orm.Session) -> N "archived": 1, "status": "Model archived", } - assert session.get(ModelTable, SAMPLE_UUID).archived == 1 + model_row = session.get(ModelTable, SAMPLE_UUID) + assert model_row is not None + assert model_row.archived == 1 assert client.patch(base + "sample_model_for_school_1/archive").status_code == 409 From d134b8a709a8e5c69dec38c7d0896cf0b2838589 Mon Sep 17 00:00:00 2001 From: Vishakh Pillai Date: Tue, 21 Jul 2026 10:51:15 -0400 Subject: [PATCH 05/10] fix: type check --- src/webapp/databricks_test.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/webapp/databricks_test.py b/src/webapp/databricks_test.py index 14c38e9c..e0cb0f2f 100644 --- a/src/webapp/databricks_test.py +++ b/src/webapp/databricks_test.py @@ -308,7 +308,9 @@ def test_build_shared_inference_job_parameters_shape() -> None: ) -def test_download_bronze_training_inputs_rejects_path_traversal(ctrl) -> None: +def test_download_bronze_training_inputs_rejects_path_traversal( + ctrl: DatabricksControl, +) -> None: with pytest.raises(ValueError, match="basename"): ctrl.download_bronze_training_inputs_file("School", "../dataio.py") with pytest.raises(ValueError, match="basename"): @@ -318,7 +320,7 @@ def test_download_bronze_training_inputs_rejects_path_traversal(ctrl) -> None: def test_download_bronze_training_inputs_file_builds_training_inputs_path( - monkeypatch: pytest.MonkeyPatch, ctrl + monkeypatch: pytest.MonkeyPatch, ctrl: DatabricksControl ) -> None: import src.webapp.databricks as db_mod From bee76c7c3332ad31ef006c4b0226bc323e840c24 Mon Sep 17 00:00:00 2001 From: Vishakh Pillai Date: Tue, 21 Jul 2026 12:30:11 -0400 Subject: [PATCH 06/10] feat(api): apply bronze config.toml grade_map before ES course Pandera Fetch training_inputs/config.toml alongside dataio and chain resolve_es_grade_map + apply_raw_course_grade_map before the school course converter, matching ES data-audit order. Missing config soft-falls back to platform grade_map defaults. Co-authored-by: Cursor --- src/webapp/databricks.py | 6 +- src/webapp/databricks_test.py | 26 +++++++ src/webapp/validation.py | 124 +++++++++++++++++++++++++++++-- src/webapp/validation_es_test.py | 109 +++++++++++++++++++++++++++ src/webapp/validation_test.py | 4 + 5 files changed, 261 insertions(+), 8 deletions(-) diff --git a/src/webapp/databricks.py b/src/webapp/databricks.py index 3dff61ef..a6f06fb5 100644 --- a/src/webapp/databricks.py +++ b/src/webapp/databricks.py @@ -63,7 +63,7 @@ ) # Only these basenames may be read from bronze_volume/training_inputs/ (path traversal safe). -ALLOWED_BRONZE_TRAINING_INPUTS_FILES = frozenset({"dataio.py"}) +ALLOWED_BRONZE_TRAINING_INPUTS_FILES = frozenset({"dataio.py", "config.toml"}) def _create_databricks_workspace_client(operation: str) -> WorkspaceClient: @@ -649,8 +649,8 @@ def download_bronze_training_inputs_file( """ Download a file from ``bronze_volume/training_inputs/`` for an institution. - Only allowlisted basenames (currently ``dataio.py``) are permitted; nested - paths, ``..``, and absolute paths are rejected. + Only allowlisted basenames (currently ``dataio.py`` and ``config.toml``) are + permitted; nested paths, ``..``, and absolute paths are rejected. Args: inst_name: Institution display name (passed through ``databricksify_inst_name``). diff --git a/src/webapp/databricks_test.py b/src/webapp/databricks_test.py index e0cb0f2f..a7f321c9 100644 --- a/src/webapp/databricks_test.py +++ b/src/webapp/databricks_test.py @@ -345,6 +345,32 @@ def test_download_bronze_training_inputs_file_builds_training_inputs_path( ) +def test_download_bronze_training_inputs_allows_config_toml( + monkeypatch: pytest.MonkeyPatch, ctrl: DatabricksControl +) -> None: + import src.webapp.databricks as db_mod + + monkeypatch.setitem( + db_mod.databricks_vars, "DATABRICKS_HOST_URL", "https://example.databricks.com" + ) + monkeypatch.setitem(db_mod.databricks_vars, "CATALOG_NAME", "dev_catalog") + monkeypatch.setitem(db_mod.gcs_vars, "GCP_SERVICE_ACCOUNT_EMAIL", "sa@example.com") + + mock_stream = MagicMock() + mock_response = MagicMock() + mock_response.contents = mock_stream + workspace = MagicMock() + workspace.files.download.return_value = mock_response + + with mock.patch.object(db_mod, "WorkspaceClient", return_value=workspace): + result = ctrl.download_bronze_training_inputs_file("Edvise School", "config.toml") + + assert result is mock_stream + workspace.files.download.assert_called_once_with( + "/Volumes/dev_catalog/edvise_school_bronze/bronze_volume/training_inputs/config.toml" + ) + + def test_run_validated_gcs_to_bronze_sync_calls_run_now_with_bundle_params( monkeypatch: pytest.MonkeyPatch, ) -> None: diff --git a/src/webapp/validation.py b/src/webapp/validation.py index e7c63841..dfd1e36d 100644 --- a/src/webapp/validation.py +++ b/src/webapp/validation.py @@ -2,10 +2,11 @@ PDP uploads validate through Pandera schemas imported from the ``edvise`` package (optional school converters may be passed in). Edvise Schema (ES) -uploads fetch school ``training_inputs/dataio.py`` converters from the -institution bronze volume (when present), then validate via -``read_raw_es_*`` + ES Pandera schemas — parity with ES Databricks data-audit -jobs. Legacy uploads use any-format CSV read plus a PII column-name guard. +uploads fetch school ``training_inputs/dataio.py`` converters and +``training_inputs/config.toml`` grade maps from the institution bronze volume +(when present), then validate via ``read_raw_es_*`` + ES Pandera schemas — +parity with ES Databricks data-audit jobs. Legacy uploads use any-format CSV +read plus a PII column-name guard. The old API-local JSON schema validation path has been removed. """ @@ -37,6 +38,10 @@ import pandas as pd from pandera.errors import SchemaError, SchemaErrors +from edvise.data_audit.raw_course_grade_map import ( + apply_raw_course_grade_map, + resolve_es_grade_map, +) from edvise.dataio.read import ( read_raw_es_cohort_data, read_raw_es_course_data, @@ -386,6 +391,110 @@ def load_es_converters_from_bronze( return cohort_converter, course_converter +def load_es_institution_grade_map_from_bronze( + inst_name: str, +) -> Optional[Dict[str, str]]: + """ + Download ``training_inputs/config.toml`` and return institution ``grade_map``. + + Soft-falls back to ``None`` (caller should still apply platform defaults via + ``resolve_es_grade_map``) when config is missing or unreadable — parity with + jobs that always merge defaults even without school overrides. + """ + from edvise.configs.es import ESProjectConfig + from edvise.dataio.read import read_config + + from .databricks import DatabricksControl + + tmp_path: Optional[str] = None + try: + stream = DatabricksControl().download_bronze_training_inputs_file( + inst_name, relative_path="config.toml" + ) + raw = _read_stream_to_bytes(stream) + fd, tmp_path = tempfile.mkstemp(suffix="_config.toml", prefix="es_bronze_") + try: + os.write(fd, raw) + finally: + os.close(fd) + + cfg = read_config(file_path=tmp_path, schema=ESProjectConfig) + pre = getattr(cfg, "preprocessing", None) + features = getattr(pre, "features", None) if pre is not None else None + grade_map = getattr(features, "grade_map", None) if features is not None else None + if grade_map: + logger.info( + "Loaded ES institution grade_map from bronze config for institution=%s " + "(%s entries)", + inst_name, + len(grade_map), + ) + else: + logger.info( + "ES bronze config.toml for institution=%s has no grade_map; " + "using platform defaults only", + inst_name, + ) + return cast(Optional[Dict[str, str]], grade_map) + except Exception as e: + logger.warning( + "ES bronze config.toml unavailable for institution=%s; using platform " + "grade_map defaults only: %s", + inst_name, + e, + ) + return None + finally: + if tmp_path is not None: + try: + os.unlink(tmp_path) + except OSError: + pass + + +def _chain_es_course_converters( + course_converter_func: PDPConverterFunc, + grade_map: Dict[str, str], +) -> Callable[[pd.DataFrame], pd.DataFrame]: + """ + Match ES data-audit order: config grade_map first, then school dataio converter. + + Always returns a callable because ``resolve_es_grade_map`` yields at least + platform defaults. + """ + + def _course_converter_chain(df: pd.DataFrame) -> pd.DataFrame: + df = apply_raw_course_grade_map(df, grade_map) + if course_converter_func is not None: + df = course_converter_func(df) + return df + + return _course_converter_chain + + +def resolve_es_course_converter_for_upload( + inst_name: Optional[str], + course_converter_func: PDPConverterFunc, +) -> Callable[[pd.DataFrame], pd.DataFrame]: + """ + Build the course converter used at ES upload time (job parity). + + Loads institution ``grade_map`` from bronze ``config.toml`` when ``inst_name`` + is set (soft-fallback to platform defaults), then chains grade map + optional + ``dataio.converter_func_course``. + """ + institution_map: Optional[Dict[str, str]] = None + if inst_name: + institution_map = load_es_institution_grade_map_from_bronze(inst_name) + else: + logger.warning( + "ES course validation without institution_identifier; using platform " + "grade_map defaults only" + ) + grade_map = resolve_es_grade_map(institution_map) + return _chain_es_course_converters(course_converter_func, grade_map) + + def _validate_pdp_converter_callables( pdp_cohort_converter_func: PDPConverterFunc, pdp_course_converter_func: PDPConverterFunc, @@ -886,7 +995,7 @@ def validate_dataset( model_list = _model_list_from_models(models) - # ES: bronze dataio converters (soft-fallback if missing) + read_raw_es_* + Pandera. + # ES: bronze dataio + config.toml grade_map (soft-fallback) + read_raw_es_* + Pandera. if institution_id == "edvise": schema_class = pdp_edvise.get_edvise_schema_for_upload( institution_id, model_list @@ -912,6 +1021,11 @@ def validate_dataset( "ES validation without institution_identifier; validating without " "bronze dataio converters" ) + model_set = {str(m).strip().upper() for m in model_list if m} + if model_set == {"COURSE"}: + course_converter = resolve_es_course_converter_for_upload( + institution_identifier, course_converter + ) return _validate_edvise_with_repo_schema( filename, enc, diff --git a/src/webapp/validation_es_test.py b/src/webapp/validation_es_test.py index 3c453061..61df2912 100644 --- a/src/webapp/validation_es_test.py +++ b/src/webapp/validation_es_test.py @@ -14,8 +14,11 @@ from src.webapp.validation import ( HardValidationError, + _chain_es_course_converters, _import_dataio_module_isolated, load_es_converters_from_bronze, + load_es_institution_grade_map_from_bronze, + resolve_es_course_converter_for_upload, validate_file_reader, ) @@ -269,6 +272,9 @@ def test_pdp_routing_unchanged_does_not_load_es_converters(tmp_path: Path) -> No patch( "src.webapp.validation.load_es_converters_from_bronze", ) as mock_load, + patch( + "src.webapp.validation.load_es_institution_grade_map_from_bronze", + ) as mock_grade, patch( "src.webapp.validation._validate_pdp_with_edvise_read", return_value={ @@ -288,3 +294,106 @@ def test_pdp_routing_unchanged_does_not_load_es_converters(tmp_path: Path) -> No assert result["validation_status"] == "passed" mock_load.assert_not_called() + mock_grade.assert_not_called() + + +def test_chain_es_course_converters_applies_grade_map_before_dataio() -> None: + """Job parity: grade_map runs first, then school course converter.""" + order: list[str] = [] + + def school_converter(df: pd.DataFrame) -> pd.DataFrame: + order.append("dataio") + assert list(df["grade"]) == ["W"] # already mapped + return df + + chain = _chain_es_course_converters(school_converter, {"W1": "W"}) + out = chain(pd.DataFrame({"grade": ["W1"]})) + assert order == ["dataio"] + assert list(out["grade"]) == ["W"] + + +def test_load_es_institution_grade_map_missing_config_soft_fallback() -> None: + """Missing bronze config.toml soft-falls back to None (defaults applied by resolve).""" + with patch( + "src.webapp.databricks.DatabricksControl.download_bronze_training_inputs_file", + side_effect=ValueError("missing config"), + ): + assert load_es_institution_grade_map_from_bronze("school_x") is None + + +def test_load_es_institution_grade_map_from_config_bytes() -> None: + """Parses preprocessing.features.grade_map from bronze config.toml.""" + from types import SimpleNamespace + + fake_cfg = SimpleNamespace( + preprocessing=SimpleNamespace( + features=SimpleNamespace(grade_map={"W1": "W", "NC": "NR"}) + ) + ) + with ( + patch( + "src.webapp.databricks.DatabricksControl.download_bronze_training_inputs_file", + return_value=io.BytesIO(b"placeholder = true\n"), + ), + patch("edvise.dataio.read.read_config", return_value=fake_cfg), + ): + grade_map = load_es_institution_grade_map_from_bronze("school_y") + + assert grade_map == {"W1": "W", "NC": "NR"} + + +def test_es_course_upload_chains_grade_map_into_read_raw_es(tmp_path: Path) -> None: + """COURSE upload passes a chained converter (grade_map + dataio) to read_raw_es_*.""" + csv_path = tmp_path / "course.csv" + csv_path.write_text("learner_id,grade\ns1,W1\n", encoding="utf-8") + + def school_converter(df: pd.DataFrame) -> pd.DataFrame: + return df.assign(from_dataio=True) + + captured: dict[str, Any] = {} + + def fake_read(**kwargs: Any) -> pd.DataFrame: + captured["converter_func"] = kwargs.get("converter_func") + conv = kwargs.get("converter_func") + assert conv is not None + result = conv(pd.DataFrame({"learner_id": ["s1"], "grade": ["W1"]})) + captured["after_converter"] = result + return result + + with ( + patch( + "src.webapp.validation.load_es_converters_from_bronze", + return_value=(None, school_converter), + ), + patch( + "src.webapp.validation.load_es_institution_grade_map_from_bronze", + return_value={"W1": "W"}, + ), + patch( + "src.webapp.validation.read_raw_es_course_data", + side_effect=fake_read, + ), + ): + result = validate_file_reader( + str(csv_path), + ["COURSE"], + institution_id="edvise", + institution_identifier="edvise_school", + ) + + assert result["validation_status"] == "passed" + assert captured["converter_func"] is not school_converter + assert list(captured["after_converter"]["grade"]) == ["W"] + assert bool(captured["after_converter"]["from_dataio"].iloc[0]) is True + + +def test_resolve_es_course_converter_uses_defaults_when_config_missing() -> None: + """Missing config still applies platform default grade_map (e.g. W1 -> W).""" + with patch( + "src.webapp.validation.load_es_institution_grade_map_from_bronze", + return_value=None, + ): + chain = resolve_es_course_converter_for_upload("school_z", None) + + out = chain(pd.DataFrame({"grade": ["W1"]})) + assert list(out["grade"]) == ["W"] diff --git a/src/webapp/validation_test.py b/src/webapp/validation_test.py index 3448d7f8..9e5928a9 100644 --- a/src/webapp/validation_test.py +++ b/src/webapp/validation_test.py @@ -215,6 +215,10 @@ def test_validate_file_reader_edvise_course_uses_repo_schema_path( "src.webapp.validation.load_es_converters_from_bronze", return_value=(None, None), ), + patch( + "src.webapp.validation.resolve_es_course_converter_for_upload", + side_effect=lambda _inst, conv: conv, + ), patch( "src.webapp.validation._validate_edvise_with_repo_schema", return_value={ From 5da8c577b2aa3b27dff8c0c7cfbb8b98ede2e7a8 Mon Sep 17 00:00:00 2001 From: Vishakh Pillai Date: Tue, 21 Jul 2026 13:11:44 -0400 Subject: [PATCH 07/10] fix: style --- src/webapp/databricks_test.py | 4 +++- src/webapp/validation.py | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/webapp/databricks_test.py b/src/webapp/databricks_test.py index a7f321c9..a06483b6 100644 --- a/src/webapp/databricks_test.py +++ b/src/webapp/databricks_test.py @@ -363,7 +363,9 @@ def test_download_bronze_training_inputs_allows_config_toml( workspace.files.download.return_value = mock_response with mock.patch.object(db_mod, "WorkspaceClient", return_value=workspace): - result = ctrl.download_bronze_training_inputs_file("Edvise School", "config.toml") + result = ctrl.download_bronze_training_inputs_file( + "Edvise School", "config.toml" + ) assert result is mock_stream workspace.files.download.assert_called_once_with( diff --git a/src/webapp/validation.py b/src/webapp/validation.py index dfd1e36d..86cde8c3 100644 --- a/src/webapp/validation.py +++ b/src/webapp/validation.py @@ -421,7 +421,9 @@ def load_es_institution_grade_map_from_bronze( cfg = read_config(file_path=tmp_path, schema=ESProjectConfig) pre = getattr(cfg, "preprocessing", None) features = getattr(pre, "features", None) if pre is not None else None - grade_map = getattr(features, "grade_map", None) if features is not None else None + grade_map = ( + getattr(features, "grade_map", None) if features is not None else None + ) if grade_map: logger.info( "Loaded ES institution grade_map from bronze config for institution=%s " From 1ba80c5047f620bd397a17d7db7c9a0769974d30 Mon Sep 17 00:00:00 2001 From: Vishakh Pillai <64162993+vishpillai123@users.noreply.github.com> Date: Wed, 22 Jul 2026 07:09:32 -0400 Subject: [PATCH 08/10] feat: add archived field to /models endpoint responses (#275) Expose the existing model.archived column on ModelInfo so that GET /{inst_id}/models, GET /{inst_id}/models/{model_name}, and POST /{inst_id}/models/ all report whether a model is archived, instead of only the PATCH .../archive endpoint knowing about it. Co-authored-by: Vishakh Pillai Co-authored-by: Cursor --- src/webapp/routers/models.py | 4 ++++ src/webapp/routers/models_test.py | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/webapp/routers/models.py b/src/webapp/routers/models.py index e7393e21..8fe71772 100644 --- a/src/webapp/routers/models.py +++ b/src/webapp/routers/models.py @@ -218,6 +218,7 @@ class ModelInfo(BaseModel): created_by: str | None = None valid: bool = True deleted: bool | None = None + archived: bool = False def _model_version_as_str(version: Any) -> str | None: @@ -297,6 +298,7 @@ def read_inst_models( "created_by": uuid_to_str(elem[0].created_by), "deleted": elem[0].deleted, "valid": elem[0].valid, + "archived": bool(elem[0].archived), } ) return res @@ -373,6 +375,7 @@ def create_model( "created_by": uuid_to_str(query_result[0][0].created_by), "deleted": query_result[0][0].deleted, "valid": query_result[0][0].valid, + "archived": bool(query_result[0][0].archived), } @@ -420,6 +423,7 @@ def read_inst_model( "created_by": uuid_to_str(query_result[0][0].created_by), "deleted": query_result[0][0].deleted, "valid": query_result[0][0].valid, + "archived": bool(query_result[0][0].archived), } diff --git a/src/webapp/routers/models_test.py b/src/webapp/routers/models_test.py index 7911fd76..3a32212e 100644 --- a/src/webapp/routers/models_test.py +++ b/src/webapp/routers/models_test.py @@ -63,6 +63,7 @@ def same_model_orderless(a_elem: ModelInfo, b_elem: ModelInfo) -> bool: or a_elem.m_id != b_elem.m_id or a_elem.valid != b_elem.valid or a_elem.deleted != b_elem.deleted + or a_elem.archived != b_elem.archived ): return False return True @@ -247,6 +248,7 @@ def test_read_inst_models(client: TestClient) -> None: inst_id="1d7c75c33eda42949c6675ea8af97b55", deleted=None, valid=True, + archived=False, ), ) @@ -279,6 +281,7 @@ def test_read_inst_model(client: TestClient) -> None: m_id="e4862c62829440d8ab4c9c298f02f619", name="sample_model_for_school_1", valid=True, + archived=False, ) assert same_model_orderless(response_model, expected_model) @@ -301,6 +304,21 @@ def test_archive_model(client: TestClient, session: sqlalchemy.orm.Session) -> N assert model_row.archived == 1 assert client.patch(base + "sample_model_for_school_1/archive").status_code == 409 + # Confirm the archived state is reflected on the model read endpoints. + get_response = client.get(base + "sample_model_for_school_1") + assert get_response.status_code == 200 + assert get_response.json()["archived"] is True + + list_response = client.get( + "/institutions/" + uuid_to_str(USER_VALID_INST_UUID) + "/models" + ) + assert list_response.status_code == 200 + assert next( + m["archived"] + for m in list_response.json() + if m["name"] == "sample_model_for_school_1" + ) + def test_read_inst_model_outputs(client: TestClient) -> None: """Test GET /institutions/345/models/10/output.""" From 93a62a1008fa299055c2719d8aac8d2199cc75c8 Mon Sep 17 00:00:00 2001 From: William Carr Date: Wed, 22 Jul 2026 07:14:37 -0400 Subject: [PATCH 09/10] chore(release): bump version 1.3.0 --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 9e14d2f2..388f480a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "src" -version = "1.2.0" +version = "1.3.0" description = "School-agnostic lib for implementing Edvise workflows." readme = "README.md" requires-python = ">=3.12,<3.13" From f61de68ecccca6bbccaf422844395334fd8f2f73 Mon Sep 17 00:00:00 2001 From: William Carr Date: Wed, 22 Jul 2026 07:14:58 -0400 Subject: [PATCH 10/10] chore(release): update CHANGELOG for 1.3.0 --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb6ee555..7611414d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## 1.3.0 (2026-07-22) +- feat: validate ES uploads with bronze `dataio` converters and Pandera (#273) +- feat: apply bronze `config.toml` `grade_map` before ES course Pandera (#273) +- feat: add PATCH endpoint to archive models (`/{inst_id}/models/{model_name}/archive`) (#274) +- feat: expose `archived` on `/models` endpoint responses (#275) + ## 1.2.0 (2026-07-15) - feat: list institution bronze volume CSVs via `/input/bronze-datasets` (#206) - feat: import a bronze volume dataset into GCS via `/input/upload-from-volume-to-gcs-bucket` (#206)