fix(tasks): copy-on-write su modifica/cancellazione e ordine tolleranze (#4, #26)

#4 - api/routers/tasks.py: update_task, delete_task, create_subtask,
update_subtask, delete_subtask controllavano solo is_current, non
version_has_measurements: la versione con misure veniva modificata sul posto,
cambiando i limiti di quote gia' giudicate pass/fail. Nuovi helper
_task_for_write / _subtask_for_write riusano recipe_service.create_new_version
(lo stesso copy-on-write di create_task): se la versione ha misure creano una
nuova versione, ritrovano il task per order_index e la quota per marker_number
(chiavi che _copy_tasks_to_version preserva), e la scrittura va sulla copia.
La versione misurata resta intatta.
Trovato e corretto nello stesso punto: il task ritrovato nella copia non ha
.version precaricato (new_version.tasks viene da selectinload, che non popola
il backref figlio->genitore), e TaskResponse.recipe_id legge task.version.recipe_id
- senza assegnare esplicitamente candidate.version = new_version, update_task
va in MissingGreenlet (lazy load fuori da un await) invece di rispondere 200.

#26 - models/api/task.py: nuovo validate_tolerance_order (ltl<=lwl<=nominal<=
uwl<=utl sui soli campi valorizzati), usato come model_validator su
SubtaskCreate e richiamato dal router in update_subtask sui valori risultanti
(esistenti + parziali) prima del flush, perche' SubtaskUpdate non ha un insieme
fisso di campi da validare da solo.

Test: src/backend/tests/test_tasks.py, TestCopyOnWriteOnEdit (5) e
TestToleranceOrder (4); adattato test_update_subtask esistente (nominal 12.0
violava il nuovo ordine contro utl=10.5 invariato).
PROVA: uv run --extra server --extra dev pytest src/backend/tests/test_tasks.py -q -> 16 passed

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

src/backend/api/routers/tasks.py

@@ -17,6 +17,7 @@ from src.backend.models.api.task import (
     TaskReorderRequest,
     TaskResponse,
     TaskUpdate,
+    validate_tolerance_order,
 )
 from src.backend.services import recipe_service
 
@@ -99,6 +100,64 @@ async def _ensure_version_is_current(db: AsyncSession, version_id: int) -> Recip
     return version
 
 
+async def _task_for_write(db: AsyncSession, task: RecipeTask, user: User) -> RecipeTask:
+    """Return the task to write to: the same one, or its copy in a new version.
+
+    A version that already has measurements cannot be edited on the spot - its
+    recorded pass/fail would silently start meaning something else. Copy-on-write
+    (the same one create_task/update_recipe use) gives the edit a fresh version
+    and leaves the measured one exactly as it was when the readings were taken.
+    The task is matched in the copy by its position (order_index), which
+    _copy_tasks_to_version preserves exactly.
+    """
+    if not await recipe_service.version_has_measurements(db, task.version_id):
+        return task
+
+    recipe_id = task.recipe_id
+    if recipe_id is None:
+        raise HTTPException(
+            status_code=status.HTTP_409_CONFLICT,
+            detail="Cannot determine the recipe for this task",
+        )
+    from src.backend.models.api.recipe import RecipeUpdate
+    new_version = await recipe_service.create_new_version(
+        db, recipe_id, RecipeUpdate(change_notes=f"Edited task: {task.title}"), user,
+    )
+    for candidate in new_version.tasks:
+        if candidate.order_index == task.order_index:
+            # TaskResponse.recipe_id reads task.version.recipe_id; new_version's
+            # own tasks are loaded via selectinload, which does not also
+            # populate the child->parent backref, so left alone this would
+            # lazy-load .version outside of an awaited context and crash.
+            candidate.version = new_version
+            return candidate
+    raise HTTPException(
+        status_code=status.HTTP_409_CONFLICT,
+        detail="Could not locate the task in the new version",
+    )
+
+
+async def _subtask_for_write(
+    db: AsyncSession, task: RecipeTask, subtask: RecipeSubtask, user: User,
+) -> tuple[RecipeTask, RecipeSubtask]:
+    """Return (task, subtask) to write to: the same ones, or their copies.
+
+    Same copy-on-write as _task_for_write, one level down: once the task's copy
+    is found, the quote is matched inside it by marker_number (unique per task).
+    """
+    new_task = await _task_for_write(db, task, user)
+    if new_task is task:
+        return task, subtask
+
+    for candidate in new_task.subtasks:
+        if candidate.marker_number == subtask.marker_number:
+            return new_task, candidate
+    raise HTTPException(
+        status_code=status.HTTP_409_CONFLICT,
+        detail="Could not locate the quote in the new version",
+    )
+
+
 # ---------------------------------------------------------------------------
 # Task endpoints
 # ---------------------------------------------------------------------------
@@ -264,10 +323,16 @@ async def reorder_tasks(
 async def update_task(
     task_id: int,
     data: TaskUpdate,
-    _user: User = Depends(require_maker),
+    user: User = Depends(require_maker),
     db: AsyncSession = Depends(get_db),
 ):
-    """Update a task. The task must belong to the current version. Requires Maker role."""
+    """Update a task. The task must belong to the current version. Requires Maker role.
+
+    If that version already has measurements the edit goes through copy-on-write:
+    a new version is created and its copy of the task is the one changed. The
+    version with measurements is left untouched, so its recorded pass/fail keeps
+    meaning what it meant when it was taken.
+    """
     task = await _get_task_or_404(db, task_id)
     await _ensure_version_is_current(db, task.version_id)
 
@@ -275,6 +340,8 @@ async def update_task(
     if not update_data:
         return TaskResponse.model_validate(task)
 
+    task = await _task_for_write(db, task, user)
+
     for field, value in update_data.items():
         setattr(task, field, value)
 
@@ -286,16 +353,19 @@ async def update_task(
 @router.delete("/api/tasks/{task_id}", status_code=204)
 async def delete_task(
     task_id: int,
-    _user: User = Depends(require_maker),
+    user: User = Depends(require_maker),
     db: AsyncSession = Depends(get_db),
 ):
     """Delete a task and its subtasks. The task must belong to the current version.
 
-    Requires Maker role.
+    Requires Maker role. Copy-on-write applies as in update_task: a version with
+    measurements is never touched, the deletion lands on its copy instead.
     """
     task = await _get_task_or_404(db, task_id)
     await _ensure_version_is_current(db, task.version_id)
 
+    task = await _task_for_write(db, task, user)
+
     await db.delete(task)
     await db.flush()
 
@@ -312,16 +382,19 @@ async def delete_task(
 async def create_subtask(
     task_id: int,
     data: SubtaskCreate,
-    _user: User = Depends(require_maker),
+    user: User = Depends(require_maker),
     db: AsyncSession = Depends(get_db),
 ):
     """Add a subtask to a task. The task must belong to the current version.
 
-    Requires Maker role.
+    Requires Maker role. Copy-on-write applies as in create_task: a version with
+    measurements gets a new version, and the subtask is added there instead.
     """
     task = await _get_task_or_404(db, task_id)
     await _ensure_version_is_current(db, task.version_id)
 
+    task = await _task_for_write(db, task, user)
+
     # Check marker_number uniqueness within the task
     existing_markers = {s.marker_number for s in task.subtasks}
     if data.marker_number in existing_markers:
@@ -331,7 +404,7 @@ async def create_subtask(
         )
 
     subtask = RecipeSubtask(
-        task_id=task_id,
+        task_id=task.id,
         marker_number=data.marker_number,
         description=data.description,
         measurement_type=data.measurement_type,
@@ -354,12 +427,13 @@ async def create_subtask(
 async def update_subtask(
     subtask_id: int,
     data: SubtaskUpdate,
-    _user: User = Depends(require_maker),
+    user: User = Depends(require_maker),
     db: AsyncSession = Depends(get_db),
 ):
     """Update a subtask. Its parent task must belong to the current version.
 
-    Requires Maker role.
+    Requires Maker role. Copy-on-write applies as in update_task, one level
+    down: a version with measurements is never touched, the quote's copy is.
     """
     subtask = await _get_subtask_or_404(db, subtask_id)
 
@@ -371,6 +445,24 @@ async def update_subtask(
     if not update_data:
         return SubtaskResponse.model_validate(subtask)
 
+    # SubtaskUpdate is partial, so the order can only be checked against the
+    # values the write would actually leave in place: existing + incoming.
+    def _resulting(field: str) -> float | None:
+        value = update_data.get(field, getattr(subtask, field))
+        return None if value is None else float(value)
+
+    try:
+        validate_tolerance_order(
+            _resulting("ltl"), _resulting("lwl"), _resulting("nominal"),
+            _resulting("uwl"), _resulting("utl"),
+        )
+    except ValueError as exc:
+        raise HTTPException(
+            status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, detail=str(exc),
+        )
+
+    task, subtask = await _subtask_for_write(db, task, subtask, user)
+
     for field, value in update_data.items():
         setattr(subtask, field, value)
 
@@ -382,12 +474,13 @@ async def update_subtask(
 @router.delete("/api/subtasks/{subtask_id}", status_code=204)
 async def delete_subtask(
     subtask_id: int,
-    _user: User = Depends(require_maker),
+    user: User = Depends(require_maker),
     db: AsyncSession = Depends(get_db),
 ):
     """Delete a subtask. Its parent task must belong to the current version.
 
-    Requires Maker role.
+    Requires Maker role. Copy-on-write applies as in update_task, one level
+    down: a version with measurements is never touched, the quote's copy is.
     """
     subtask = await _get_subtask_or_404(db, subtask_id)
 
@@ -395,5 +488,7 @@ async def delete_subtask(
     task = await _get_task_or_404(db, subtask.task_id)
     await _ensure_version_is_current(db, task.version_id)
 
+    task, subtask = await _subtask_for_write(db, task, subtask, user)
+
     await db.delete(subtask)
     await db.flush()

src/backend/models/api/task.py

@@ -1,7 +1,7 @@
 """Pydantic schemas for RecipeTask and RecipeSubtask operations."""
 from typing import Any, Literal, Optional
 
-from pydantic import BaseModel, ConfigDict, Field
+from pydantic import BaseModel, ConfigDict, Field, model_validator
 
 from src.backend.models.orm.task import TASK_TYPES
 
@@ -9,6 +9,31 @@ from src.backend.models.orm.task import TASK_TYPES
 TaskType = Literal[TASK_TYPES]
 
 
+def validate_tolerance_order(
+    ltl: Optional[float],
+    lwl: Optional[float],
+    nominal: Optional[float],
+    uwl: Optional[float],
+    utl: Optional[float],
+) -> None:
+    """ltl <= lwl <= nominal <= uwl <= utl, checked only across the values set.
+
+    A one-sided tolerance - only utl, say - is not an error: what is wrong is
+    limits that invert the sequence they do share, like utl landing below ltl.
+    Shared by SubtaskCreate's own validator and by the router's partial-update
+    check (SubtaskUpdate has no fixed set of fields to compare).
+    """
+    ordered = [
+        ("ltl", ltl), ("lwl", lwl), ("nominal", nominal), ("uwl", uwl), ("utl", utl),
+    ]
+    present = [(name, value) for name, value in ordered if value is not None]
+    for (prev_name, prev), (name, value) in zip(present, present[1:]):
+        if prev > value:
+            raise ValueError(
+                f"Tolerance limits out of order: {prev_name}={prev} > {name}={value}"
+            )
+
+
 class SubtaskCreate(BaseModel):
     """Schema for creating a subtask."""
     marker_number: int = Field(..., ge=1)
@@ -23,6 +48,11 @@ class SubtaskCreate(BaseModel):
     image_path: Optional[str] = Field(None, max_length=500)
     vision_output: Optional[str] = None
 
+    @model_validator(mode="after")
+    def _check_tolerance_order(self) -> "SubtaskCreate":
+        validate_tolerance_order(self.ltl, self.lwl, self.nominal, self.uwl, self.utl)
+        return self
+
 
 class SubtaskUpdate(BaseModel):
     """Schema for updating a subtask."""

src/backend/tests/test_tasks.py

@@ -7,6 +7,14 @@ from src.backend.models.orm.user import User
 from src.backend.tests.conftest import auth_headers, create_test_recipe
 
 
+async def _measure(client, user, subtask_id, version_id, value=10.0):
+    return await client.post(
+        "/api/measurements/",
+        headers=auth_headers(user),
+        json={"subtask_id": subtask_id, "version_id": version_id, "value": value},
+    )
+
+
 class TestListTasks:
     """GET /api/recipes/{recipe_id}/tasks tests."""
 
@@ -171,7 +179,9 @@ class TestUpdateSubtask:
         resp = await client.put(
             f"/api/subtasks/{subtask_id}",
             headers=auth_headers(maker_user),
-            json={"description": "Updated description", "nominal": 12.0},
+            # nominal stays inside the fixture's own lwl(9.7)/uwl(10.3): #26
+            # rejects a nominal like 12.0 that would land past utl(10.5).
+            json={"description": "Updated description", "nominal": 10.2},
         )
         assert resp.status_code == 200
         assert resp.json()["description"] == "Updated description"
@@ -221,3 +231,266 @@ class TestReorderTasks:
         assert resp.status_code == 200
         reordered = resp.json()
         assert [t["id"] for t in reordered] == reversed_ids
+
+
+class TestCopyOnWriteOnEdit:
+    """A version with measurements is never edited in place (gitea#4)."""
+
+    async def test_update_task_with_measurements_creates_new_version(
+        self,
+        client: AsyncClient,
+        maker_user: User,
+        measurement_tec_user: User,
+        db_session: AsyncSession,
+    ):
+        recipe = await create_test_recipe(db_session, maker_user.id, code="COW-UT")
+        task = (await client.get(
+            f"/api/recipes/{recipe.id}/tasks", headers=auth_headers(maker_user),
+        )).json()[0]
+        task_id, old_version_id = task["id"], task["version_id"]
+        subtask_id = task["subtasks"][0]["id"]
+
+        assert (await _measure(
+            client, measurement_tec_user, subtask_id, old_version_id
+        )).status_code == 200
+
+        resp = await client.put(
+            f"/api/tasks/{task_id}",
+            headers=auth_headers(maker_user),
+            json={"title": "Titolo nuovo"},
+        )
+        assert resp.status_code == 200, resp.text
+        assert resp.json()["version_id"] != old_version_id
+        assert resp.json()["title"] == "Titolo nuovo"
+
+        # The measured version keeps its original title and its versions count grew.
+        old = (await client.get(
+            f"/api/recipes/{recipe.id}/versions/1", headers=auth_headers(maker_user),
+        )).json()
+        assert old["tasks"][0]["title"] == "Test Task"
+        versions = (await client.get(
+            f"/api/recipes/{recipe.id}/versions", headers=auth_headers(maker_user),
+        )).json()
+        assert len(versions) == 2
+
+        # The measurement is still findable under the old version.
+        listed = (await client.get(
+            "/api/measurements/", headers=auth_headers(measurement_tec_user),
+            params={"version_id": old_version_id},
+        )).json()
+        assert listed["total"] == 1
+
+    async def test_delete_task_with_measurements_creates_new_version(
+        self,
+        client: AsyncClient,
+        maker_user: User,
+        measurement_tec_user: User,
+        db_session: AsyncSession,
+    ):
+        recipe = await create_test_recipe(db_session, maker_user.id, code="COW-DT")
+        task = (await client.get(
+            f"/api/recipes/{recipe.id}/tasks", headers=auth_headers(maker_user),
+        )).json()[0]
+        task_id, old_version_id = task["id"], task["version_id"]
+        subtask_id = task["subtasks"][0]["id"]
+
+        assert (await _measure(
+            client, measurement_tec_user, subtask_id, old_version_id
+        )).status_code == 200
+
+        resp = await client.delete(
+            f"/api/tasks/{task_id}", headers=auth_headers(maker_user),
+        )
+        assert resp.status_code == 204
+
+        # The measured version still has its task untouched.
+        old = (await client.get(
+            f"/api/recipes/{recipe.id}/versions/1", headers=auth_headers(maker_user),
+        )).json()
+        assert len(old["tasks"]) == 1
+
+        # The new current version has the task removed. expunge_all forces a
+        # fresh read - the shared test session otherwise still has the old
+        # RecipeVersion.tasks collection cached from create_test_recipe.
+        db_session.expunge_all()
+        current = (await client.get(
+            f"/api/recipes/{recipe.id}/tasks", headers=auth_headers(maker_user),
+        )).json()
+        assert current == []
+
+    async def test_update_subtask_with_measurements_creates_new_version(
+        self,
+        client: AsyncClient,
+        maker_user: User,
+        measurement_tec_user: User,
+        db_session: AsyncSession,
+    ):
+        recipe = await create_test_recipe(db_session, maker_user.id, code="COW-US")
+        task = (await client.get(
+            f"/api/recipes/{recipe.id}/tasks", headers=auth_headers(maker_user),
+        )).json()[0]
+        old_version_id = task["version_id"]
+        subtask = task["subtasks"][0]
+        subtask_id = subtask["id"]
+
+        assert (await _measure(
+            client, measurement_tec_user, subtask_id, old_version_id
+        )).status_code == 200
+
+        resp = await client.put(
+            f"/api/subtasks/{subtask_id}",
+            headers=auth_headers(maker_user),
+            json={"description": "Quota rivista"},
+        )
+        assert resp.status_code == 200, resp.text
+        assert resp.json()["description"] == "Quota rivista"
+        assert resp.json()["id"] != subtask_id, "la modifica va sulla copia, non sull'originale"
+
+        # The measured version's own quote is untouched.
+        old = (await client.get(
+            f"/api/recipes/{recipe.id}/versions/1", headers=auth_headers(maker_user),
+        )).json()
+        assert old["tasks"][0]["subtasks"][0]["description"] == subtask["description"]
+
+    async def test_delete_subtask_with_measurements_creates_new_version(
+        self,
+        client: AsyncClient,
+        maker_user: User,
+        measurement_tec_user: User,
+        db_session: AsyncSession,
+    ):
+        recipe = await create_test_recipe(db_session, maker_user.id, code="COW-DS")
+        task = (await client.get(
+            f"/api/recipes/{recipe.id}/tasks", headers=auth_headers(maker_user),
+        )).json()[0]
+        old_version_id = task["version_id"]
+        subtask_id = task["subtasks"][0]["id"]
+
+        assert (await _measure(
+            client, measurement_tec_user, subtask_id, old_version_id
+        )).status_code == 200
+
+        resp = await client.delete(
+            f"/api/subtasks/{subtask_id}", headers=auth_headers(maker_user),
+        )
+        assert resp.status_code == 204
+
+        # The measured version keeps its quote.
+        old = (await client.get(
+            f"/api/recipes/{recipe.id}/versions/1", headers=auth_headers(maker_user),
+        )).json()
+        assert len(old["tasks"][0]["subtasks"]) == 1
+
+        # The new current version has no quotes left on the task. expunge_all
+        # forces a fresh read past the shared test session's cached collection.
+        db_session.expunge_all()
+        current = (await client.get(
+            f"/api/recipes/{recipe.id}/tasks", headers=auth_headers(maker_user),
+        )).json()
+        assert current[0]["subtasks"] == []
+
+    async def test_create_subtask_with_measurements_creates_new_version(
+        self,
+        client: AsyncClient,
+        maker_user: User,
+        measurement_tec_user: User,
+        db_session: AsyncSession,
+    ):
+        recipe = await create_test_recipe(db_session, maker_user.id, code="COW-CS")
+        task = (await client.get(
+            f"/api/recipes/{recipe.id}/tasks", headers=auth_headers(maker_user),
+        )).json()[0]
+        task_id, old_version_id = task["id"], task["version_id"]
+        subtask_id = task["subtasks"][0]["id"]
+
+        assert (await _measure(
+            client, measurement_tec_user, subtask_id, old_version_id
+        )).status_code == 200
+
+        resp = await client.post(
+            f"/api/tasks/{task_id}/subtasks",
+            headers=auth_headers(maker_user),
+            json={"marker_number": 9, "description": "Nuova quota", "utl": 5.0},
+        )
+        assert resp.status_code == 201, resp.text
+
+        # The measured version's task is unchanged (still one quote).
+        old = (await client.get(
+            f"/api/recipes/{recipe.id}/versions/1", headers=auth_headers(maker_user),
+        )).json()
+        assert len(old["tasks"][0]["subtasks"]) == 1
+
+        # The new current version's task has both quotes. expunge_all forces a
+        # fresh read past the shared test session's cached collection.
+        db_session.expunge_all()
+        current = (await client.get(
+            f"/api/recipes/{recipe.id}/tasks", headers=auth_headers(maker_user),
+        )).json()
+        assert len(current[0]["subtasks"]) == 2
+
+
+class TestToleranceOrder:
+    """ltl <= lwl <= nominal <= uwl <= utl, or 422 (gitea#26)."""
+
+    async def test_create_subtask_utl_below_ltl_is_422(
+        self, client: AsyncClient, maker_user: User, db_session: AsyncSession,
+    ):
+        recipe = await create_test_recipe(db_session, maker_user.id, code="TOL-A")
+        task_id = (await client.get(
+            f"/api/recipes/{recipe.id}/tasks", headers=auth_headers(maker_user),
+        )).json()[0]["id"]
+
+        resp = await client.post(
+            f"/api/tasks/{task_id}/subtasks",
+            headers=auth_headers(maker_user),
+            json={"marker_number": 5, "description": "Quota invertita", "ltl": 8, "utl": 6},
+        )
+        assert resp.status_code == 422
+
+    async def test_create_subtask_uwl_above_utl_is_422(
+        self, client: AsyncClient, maker_user: User, db_session: AsyncSession,
+    ):
+        recipe = await create_test_recipe(db_session, maker_user.id, code="TOL-B")
+        task_id = (await client.get(
+            f"/api/recipes/{recipe.id}/tasks", headers=auth_headers(maker_user),
+        )).json()[0]["id"]
+
+        resp = await client.post(
+            f"/api/tasks/{task_id}/subtasks",
+            headers=auth_headers(maker_user),
+            json={"marker_number": 5, "description": "Warning oltre tolleranza", "uwl": 11, "utl": 10},
+        )
+        assert resp.status_code == 422
+
+    async def test_create_subtask_one_sided_tolerance_is_ok(
+        self, client: AsyncClient, maker_user: User, db_session: AsyncSession,
+    ):
+        recipe = await create_test_recipe(db_session, maker_user.id, code="TOL-C")
+        task_id = (await client.get(
+            f"/api/recipes/{recipe.id}/tasks", headers=auth_headers(maker_user),
+        )).json()[0]["id"]
+
+        resp = await client.post(
+            f"/api/tasks/{task_id}/subtasks",
+            headers=auth_headers(maker_user),
+            json={"marker_number": 5, "description": "Solo utl", "utl": 10.0},
+        )
+        assert resp.status_code == 201, resp.text
+
+    async def test_update_subtask_that_would_invert_the_order_is_422(
+        self, client: AsyncClient, maker_user: User, db_session: AsyncSession,
+    ):
+        """Partial update: the new value is checked against what stays (#26)."""
+        recipe = await create_test_recipe(db_session, maker_user.id, code="TOL-D")
+        subtask_id = (await client.get(
+            f"/api/recipes/{recipe.id}/tasks", headers=auth_headers(maker_user),
+        )).json()[0]["subtasks"][0]["id"]
+
+        # Fixture quote: ltl=9.5, lwl=9.7, nominal=10.0, uwl=10.3, utl=10.5.
+        # Raising ltl past the existing utl inverts the sequence.
+        resp = await client.put(
+            f"/api/subtasks/{subtask_id}",
+            headers=auth_headers(maker_user),
+            json={"ltl": 11.0},
+        )
+        assert resp.status_code == 422