diff --git a/src/api/server.py b/src/api/server.py index bfee37e..68bf219 100644 --- a/src/api/server.py +++ b/src/api/server.py @@ -60,6 +60,8 @@ def route_request( if _is_training_progress_post(method, path): _validate_progress_identifier_body(body or {}) return 404, {"error": "not_found"} + if _is_training_attempt_create_post(method, path): + _validate_attempt_create_body(body or {}) deps = dependencies or build_api_dependencies() @@ -254,6 +256,10 @@ def _is_training_progress_post(method: str, path: str) -> bool: return method == "POST" and path == "/training/progress" +def _is_training_attempt_create_post(method: str, path: str) -> bool: + return method == "POST" and path == "/training/attempts" + + def _path_parts(path: str) -> list[str]: return [part for part in path.strip("/").split("/") if part] @@ -284,6 +290,27 @@ def _validate_progress_identifier_body(body: dict) -> None: _parse_body_identifier(body[key], key) +def _validate_attempt_create_body(body: dict) -> None: + _parse_required_json_identifier(body, "case_id") + + +def _parse_required_json_identifier(body: dict, key: str) -> int: + if not isinstance(body, dict): + raise ValueError("invalid_json_body") + if body.get(key) is None: + raise ValueError(f"missing_required_json_field:{key}") + value = body[key] + if isinstance(value, bool): + raise ValueError(f"invalid_json_field:{key}") + try: + parsed = int(value) + except (TypeError, ValueError) as exc: + raise ValueError(f"invalid_json_field:{key}") from exc + if parsed < 1: + raise ValueError(f"invalid_json_field:{key}") + return parsed + + def _parse_body_identifier(value, key: str) -> int: if isinstance(value, bool): raise ValueError(f"invalid_body_field:{key}") diff --git a/tests/unit/test_training_api_error_contract_m9.py b/tests/unit/test_training_api_error_contract_m9.py index 4bf1686..65c659f 100644 --- a/tests/unit/test_training_api_error_contract_m9.py +++ b/tests/unit/test_training_api_error_contract_m9.py @@ -269,6 +269,44 @@ class TrainingApiErrorContractM9Tests(unittest.TestCase): self.assertEqual(service.calls, []) self.assertEqual(repository.calls, []) + def test_create_attempt_rejects_invalid_case_identifier_before_side_effects(self) -> None: + post_cases = ( + ({"case_id": 0}, "invalid_json_field:case_id"), + ({"case_id": -7}, "invalid_json_field:case_id"), + ({"case_id": False}, "invalid_json_field:case_id"), + ({"case_id": True}, "invalid_json_field:case_id"), + ) + + for payload, expected_error in post_cases: + with self.subTest(payload=payload): + repository = _TrainingRepositoryStub() + service = TrainingCaseService(repository=repository) + dependencies = _training_dependencies(service) + + with self.assertRaises(ValueError) as raised: + server.route_request( + "/training/attempts", + {}, + dependencies=dependencies, + method="POST", + body=payload, + ) + + self.assertEqual(str(raised.exception), expected_error) + self.assertEqual(repository.calls, []) + + handler = _HandlerHarness( + "/training/attempts", + dependencies, + method="POST", + body=json.dumps(payload).encode("utf-8"), + ) + + handler.dispatch() + + self._assert_public_error(handler, 400, expected_error) + self.assertEqual(repository.calls, []) + def test_missing_and_invalid_attempt_fields_return_public_400(self) -> None: post_cases = ( ("/training/attempts", {}, "missing_required_json_field:case_id"), diff --git a/tests/unit/test_training_public_api_contract_m8.py b/tests/unit/test_training_public_api_contract_m8.py index b5f89b0..663b219 100644 --- a/tests/unit/test_training_public_api_contract_m8.py +++ b/tests/unit/test_training_public_api_contract_m8.py @@ -166,6 +166,7 @@ PUBLIC_VALIDATION_ERROR_CONTRACT_TEST_INVENTORY = ( "file": VALIDATION_ERROR_CONTRACT_TEST_FILE, "class_name": "TrainingApiErrorContractM9Tests", "test_names": ( + "test_create_attempt_rejects_invalid_case_identifier_before_side_effects", "test_missing_and_invalid_attempt_fields_return_public_400", "test_error_responses_are_minimal_json_and_privacy_safe", ), @@ -538,6 +539,81 @@ class _M21AttemptLifecycleService: } +class _WorkflowContractRepository: + def __init__(self) -> None: + self.calls = [] + + def get_case(self, case_id: int, **kwargs): + self.calls.append( + ( + "get_case", + { + "case_id": case_id, + "include_" + INTERNAL_FIELD: kwargs.get("include_" + INTERNAL_FIELD, False), + }, + ) + ) + return { + "id": case_id, + "case_type": "sweep", + "skill_tag": "sweep", + "status": "published", + "meta": { + "title": "Workflow contract case", + "public_note": "visible", + "answer": M21_PRIVATE_MARKERS[0], + INTERNAL_FIELD: M21_PRIVATE_MARKERS[1], + }, + INTERNAL_FIELD: {"rubric": M21_PRIVATE_MARKERS[2]}, + } + + def create_attempt( + self, + case_id: int, + user_id: str = "local", + status: str = "started", + current_frame_index: int = 0, + meta=None, + ) -> int: + self.calls.append( + ( + "create_attempt", + { + "case_id": case_id, + "user_id": user_id, + "status": status, + "current_frame_index": current_frame_index, + "meta": meta, + }, + ) + ) + return 501 + + def submit_attempt(self, attempt_id: int, total_score=None, status: str = "submitted") -> None: + self.calls.append( + ( + "submit_attempt", + {"attempt_id": attempt_id, "total_score": total_score, "status": status}, + ) + ) + + def get_progress(self, user_id: str = "local", skill_tag=None, limit: int = 100): + self.calls.append(("get_progress", {"user_id": user_id, "skill_tag": skill_tag, "limit": limit})) + return [ + { + "user_id": user_id, + "skill_tag": skill_tag or "sweep", + "attempt_count": 1, + "avg_score": 8.25, + "meta": { + "public_result": "submitted", + "answer": M21_PRIVATE_MARKERS[0], + INTERNAL_FIELD: M21_PRIVATE_MARKERS[1], + }, + } + ][:limit] + + def _documented_routes() -> tuple: text = TRAINING_PRODUCT_DOC.read_text(encoding="utf-8") routes = [] @@ -901,6 +977,97 @@ class TrainingPublicApiContractM8Tests(unittest.TestCase): self.assertEqual(handler.response_headers.get("Content-Length"), str(len(handler.wfile.getvalue()))) self.assertEqual(probe.calls, [expected_call]) + def test_training_workflow_contract_case_attempt_submit_progress_is_deterministic(self) -> None: + repository = _WorkflowContractRepository() + service = TrainingCaseService(repository=repository) + dependencies = _training_dependencies(service) + + detail_status, detail_payload = server.route_request("/training/cases/17", {}, dependencies=dependencies) + attempt_status, attempt_payload = server.route_request( + "/training/attempts", + {}, + dependencies=dependencies, + method="POST", + body={ + "case_id": 17, + "user_id": "analyst", + "current_frame_index": 3, + "meta": {"source": "workflow-contract"}, + }, + ) + submit_status, submit_payload = server.route_request( + "/training/attempts/501/submit", + {}, + dependencies=dependencies, + method="POST", + body={"total_score": 8.25}, + ) + progress_status, progress_payload = server.route_request( + "/training/progress", + {"user_id": ["analyst"], "skill_tag": ["sweep"], "limit": ["1"]}, + dependencies=dependencies, + ) + + self.assertEqual([detail_status, attempt_status, submit_status, progress_status], [200, 201, 200, 200]) + self.assertEqual( + detail_payload, + { + "id": 17, + "case_type": "sweep", + "skill_tag": "sweep", + "status": "published", + "meta": {"title": "Workflow contract case", "public_note": "visible"}, + }, + ) + self.assertEqual(attempt_payload, {"id": 501}) + self.assertEqual(submit_payload, {"id": 501, "status": "submitted"}) + self.assertEqual( + progress_payload, + { + "items": [ + { + "user_id": "analyst", + "skill_tag": "sweep", + "attempt_count": 1, + "avg_score": 8.25, + "meta": {"public_result": "submitted"}, + } + ] + }, + ) + self.assertEqual( + repository.calls, + [ + ("get_case", {"case_id": 17, "include_" + INTERNAL_FIELD: False}), + ( + "create_attempt", + { + "case_id": 17, + "user_id": "analyst", + "status": "started", + "current_frame_index": 3, + "meta": {"source": "workflow-contract"}, + }, + ), + ("submit_attempt", {"attempt_id": 501, "total_score": 8.25, "status": "submitted"}), + ("get_progress", {"user_id": "analyst", "skill_tag": "sweep", "limit": 1}), + ], + ) + + serialized = json.dumps( + { + "detail": detail_payload, + "attempt": attempt_payload, + "submit": submit_payload, + "progress": progress_payload, + }, + sort_keys=True, + ) + self.assertNotIn(INTERNAL_FIELD, serialized) + for private_text in M21_PRIVATE_MARKERS: + with self.subTest(private_text=private_text): + self.assertNotIn(private_text, serialized) + def test_dependency_builder_exposes_training_handler_functions(self) -> None: expected = { "training_cases": training_api.get_training_cases,