diff --git a/src/backend/api/routers/tasks.py b/src/backend/api/routers/tasks.py index 7d1ad1f..3a66c85 100644 --- a/src/backend/api/routers/tasks.py +++ b/src/backend/api/routers/tasks.py @@ -179,6 +179,7 @@ async def create_task( file_path=data.file_path, file_type=data.file_type, annotations_json=data.annotations_json, + vision_json=data.vision_json, ) db.add(new_task) await db.flush() @@ -197,6 +198,7 @@ async def create_task( ltl=sub_data.ltl, unit=sub_data.unit, image_path=sub_data.image_path, + vision_output=sub_data.vision_output, ) db.add(sub) @@ -340,6 +342,7 @@ async def create_subtask( ltl=data.ltl, unit=data.unit, image_path=data.image_path, + vision_output=data.vision_output, ) db.add(subtask) await db.flush() diff --git a/src/backend/services/recipe_service.py b/src/backend/services/recipe_service.py index eac16ce..1d9a363 100644 --- a/src/backend/services/recipe_service.py +++ b/src/backend/services/recipe_service.py @@ -88,6 +88,11 @@ async def _copy_tasks_to_version( file_path=task.file_path, file_type=task.file_type, annotations_json=task.annotations_json, + # Carried over explicitly, same reasoning as task_type above: a + # copy-on-write version that dropped the camera graph would leave + # a camera_measure task unable to execute from the moment a + # recipe with measurements is next edited. + vision_json=task.vision_json, ) db.add(new_task) await db.flush() # get new_task.id @@ -105,6 +110,7 @@ async def _copy_tasks_to_version( ltl=sub.ltl, unit=sub.unit, image_path=sub.image_path, + vision_output=sub.vision_output, ) db.add(new_sub) diff --git a/src/backend/tests/test_vision_copy_on_write.py b/src/backend/tests/test_vision_copy_on_write.py new file mode 100644 index 0000000..54845f2 --- /dev/null +++ b/src/backend/tests/test_vision_copy_on_write.py @@ -0,0 +1,167 @@ +"""C1: the vision graph must survive copy-on-write, and creation must save it. + +The spec assumed versioning was free for `vision_json`/`vision_output` - "un +campo in piu nella copia, non una logica nuova". `_copy_tasks_to_version` +copies an explicit field list that omitted both fields, so the first edit to +a recipe that already has measurements silently drops the camera graph from +the new version. The creation paths (`POST /api/recipes/{id}/tasks` and +`POST /api/tasks/{id}/subtasks`) have the same shape of bug: `TaskCreate` and +`SubtaskCreate` declare the fields, but the routers never pass them to the +ORM constructors, so the API answers 200 and silently does not store them. + +Each test file here reads on its own - the same idiom `test_vision_graph.py` +and `test_recipes.py` already use. +""" +import pytest +from httpx import AsyncClient +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from src.backend.models.orm.measurement import Measurement +from src.backend.models.orm.recipe import RecipeVersion +from src.backend.models.orm.task import RecipeSubtask, RecipeTask +from src.backend.models.orm.user import User +from src.backend.tests.conftest import auth_headers, create_test_recipe + + +async def _seed_measurement(db: AsyncSession, recipe_id: int, user_id: int) -> None: + """Give the current version a measurement, so an update takes the + copy-on-write path instead of updating in place.""" + version_id = (await db.execute( + select(RecipeVersion.id).where( + RecipeVersion.recipe_id == recipe_id, + RecipeVersion.is_current == True, # noqa: E712 + ) + )).scalar_one() + subtask_id = (await db.execute( + select(RecipeSubtask.id) + .join(RecipeSubtask.task) + .where(RecipeSubtask.task.has(version_id=version_id)) + )).scalar_one() + db.add(Measurement( + subtask_id=subtask_id, + version_id=version_id, + measured_by=user_id, + value=10.0, + pass_fail="pass", + deviation=0.0, + input_method="manual", + )) + await db.flush() + + +async def _current_task_and_subtask(db: AsyncSession, recipe_id: int): + version = (await db.execute( + select(RecipeVersion).where( + RecipeVersion.recipe_id == recipe_id, + RecipeVersion.is_current == True, # noqa: E712 + ) + )).scalar_one() + task = (await db.execute( + select(RecipeTask).where(RecipeTask.version_id == version.id) + )).scalars().first() + subtask = (await db.execute( + select(RecipeSubtask).where(RecipeSubtask.task_id == task.id) + )).scalars().first() + return version, task, subtask + + +@pytest.mark.asyncio +async def test_copy_on_write_preserves_the_vision_graph( + client: AsyncClient, db_session: AsyncSession, maker_user: User, +): + """A recipe edit that triggers copy-on-write must not drop the camera graph.""" + recipe = await create_test_recipe(db_session, maker_user.id) + _, task, subtask = await _current_task_and_subtask(db_session, recipe.id) + task.task_type = "camera_measure" + task.vision_json = {"schema_version": 1, "name": "p", "tools": []} + subtask.vision_output = "t1.diameter" + await db_session.flush() + + # A measurement on v1 forces the next PUT onto the copy-on-write path. + await _seed_measurement(db_session, recipe.id, maker_user.id) + db_session.expunge_all() + + resp = await client.put( + f"/api/recipes/{recipe.id}", + headers=auth_headers(maker_user), + json={"change_notes": "Edit that must not drop the graph"}, + ) + assert resp.status_code == 200 + db_session.expunge_all() + + version, new_task, new_subtask = await _current_task_and_subtask(db_session, recipe.id) + assert version.version_number == 2, "the edit should have created v2" + assert new_task.vision_json == {"schema_version": 1, "name": "p", "tools": []} + assert new_subtask.vision_output == "t1.diameter" + + +@pytest.mark.asyncio +async def test_creating_a_task_persists_its_vision_graph( + client: AsyncClient, db_session: AsyncSession, maker_user: User, +): + """POST /api/recipes/{id}/tasks must store vision_json and vision_output, + not just accept and echo them back.""" + recipe = await create_test_recipe(db_session, maker_user.id) + + resp = await client.post( + f"/api/recipes/{recipe.id}/tasks", + headers=auth_headers(maker_user), + json={ + "title": "Camera task", + "task_type": "camera_measure", + "vision_json": {"schema_version": 1, "name": "p", "tools": []}, + "subtasks": [ + { + "marker_number": 1, + "description": "Diameter", + "nominal": 10.0, + "vision_output": "t1.diameter", + } + ], + }, + ) + assert resp.status_code == 201 + body = resp.json() + assert body["vision_json"] == {"schema_version": 1, "name": "p", "tools": []} + assert body["subtasks"][0]["vision_output"] == "t1.diameter" + + # Reload from the database - the response reflects the ORM object right + # after flush, so this catches a constructor that dropped the field. + db_session.expunge_all() + task = (await db_session.execute( + select(RecipeTask).where(RecipeTask.id == body["id"]) + )).scalar_one() + sub = (await db_session.execute( + select(RecipeSubtask).where(RecipeSubtask.task_id == task.id) + )).scalars().one() + assert task.vision_json == {"schema_version": 1, "name": "p", "tools": []} + assert sub.vision_output == "t1.diameter" + + +@pytest.mark.asyncio +async def test_creating_a_subtask_persists_its_vision_output( + client: AsyncClient, db_session: AsyncSession, maker_user: User, +): + """POST /api/tasks/{id}/subtasks (the standalone endpoint) must also store it.""" + recipe = await create_test_recipe(db_session, maker_user.id) + _, task, _ = await _current_task_and_subtask(db_session, recipe.id) + + resp = await client.post( + f"/api/tasks/{task.id}/subtasks", + headers=auth_headers(maker_user), + json={ + "marker_number": 2, + "description": "Radius", + "nominal": 5.0, + "vision_output": "t1.radius", + }, + ) + assert resp.status_code == 201 + assert resp.json()["vision_output"] == "t1.radius" + + db_session.expunge_all() + sub = (await db_session.execute( + select(RecipeSubtask).where(RecipeSubtask.marker_number == 2) + )).scalar_one() + assert sub.vision_output == "t1.radius"