fix(vision): il grafo di visione sopravvive a copy-on-write e creazione
_copy_tasks_to_version copiava una lista esplicita di campi che ometteva
vision_json e vision_output: la prima modifica a una ricetta con misure
produceva una versione nuova con i task camera senza grafo. Stessa
famiglia di bug nei percorsi di creazione (POST /api/recipes/{id}/tasks,
POST /api/tasks/{id}/subtasks): i campi erano dichiarati negli schemi ma
mai passati ai costruttori ORM, quindi l'API rispondeva 200 senza
salvare nulla. TDD: test_vision_copy_on_write.py fallisce prima del fix,
passa dopo.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014BBnuACZSCJqXrMYC3LUMU
This commit is contained in:
@@ -179,6 +179,7 @@ async def create_task(
|
|||||||
file_path=data.file_path,
|
file_path=data.file_path,
|
||||||
file_type=data.file_type,
|
file_type=data.file_type,
|
||||||
annotations_json=data.annotations_json,
|
annotations_json=data.annotations_json,
|
||||||
|
vision_json=data.vision_json,
|
||||||
)
|
)
|
||||||
db.add(new_task)
|
db.add(new_task)
|
||||||
await db.flush()
|
await db.flush()
|
||||||
@@ -197,6 +198,7 @@ async def create_task(
|
|||||||
ltl=sub_data.ltl,
|
ltl=sub_data.ltl,
|
||||||
unit=sub_data.unit,
|
unit=sub_data.unit,
|
||||||
image_path=sub_data.image_path,
|
image_path=sub_data.image_path,
|
||||||
|
vision_output=sub_data.vision_output,
|
||||||
)
|
)
|
||||||
db.add(sub)
|
db.add(sub)
|
||||||
|
|
||||||
@@ -340,6 +342,7 @@ async def create_subtask(
|
|||||||
ltl=data.ltl,
|
ltl=data.ltl,
|
||||||
unit=data.unit,
|
unit=data.unit,
|
||||||
image_path=data.image_path,
|
image_path=data.image_path,
|
||||||
|
vision_output=data.vision_output,
|
||||||
)
|
)
|
||||||
db.add(subtask)
|
db.add(subtask)
|
||||||
await db.flush()
|
await db.flush()
|
||||||
|
|||||||
@@ -88,6 +88,11 @@ async def _copy_tasks_to_version(
|
|||||||
file_path=task.file_path,
|
file_path=task.file_path,
|
||||||
file_type=task.file_type,
|
file_type=task.file_type,
|
||||||
annotations_json=task.annotations_json,
|
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)
|
db.add(new_task)
|
||||||
await db.flush() # get new_task.id
|
await db.flush() # get new_task.id
|
||||||
@@ -105,6 +110,7 @@ async def _copy_tasks_to_version(
|
|||||||
ltl=sub.ltl,
|
ltl=sub.ltl,
|
||||||
unit=sub.unit,
|
unit=sub.unit,
|
||||||
image_path=sub.image_path,
|
image_path=sub.image_path,
|
||||||
|
vision_output=sub.vision_output,
|
||||||
)
|
)
|
||||||
db.add(new_sub)
|
db.add(new_sub)
|
||||||
|
|
||||||
|
|||||||
@@ -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"
|
||||||
Reference in New Issue
Block a user