32k pixel as hard group limit, better manual resolution support

tests/test_gui_convenience.py

@@ -150,6 +150,56 @@ class GuiConvenienceTests(unittest.TestCase):
         self.app.update()
         self.assertEqual(self.app.current_step.id, "fetch_batches")
 
+    def test_batch_retrieval_success_selects_integration_without_starting_it(self):
+        self.app.state_store.update_step("correction", variant="batch", last_run_variant="batch")
+        self.app._populate_tree()
+        self.app.tree.selection_set("fetch_batches")
+        self.app.update()
+        self.app.active_step_id = "fetch_batches"
+        with patch.object(self.app.runner, "start") as start:
+            self.app._finish_process(0, False)
+            self.app.update()
+        start.assert_not_called()
+        self.assertEqual(self.app.current_step.id, "correction")
+        self.assertEqual(self.app.variant_var.get(), "integrate")
+        self.assertEqual(self.app.state_store.step("correction")["variant"], "integrate")
+        self.assertIn("--deal-with-batched", self.app._make_command())
+
+    def test_integration_preserves_batch_statuses_but_invalidates_downstream_steps(self):
+        for variant in ("integrate", "batch"):
+            with self.subTest(variant=variant):
+                self.app.active_step_id = None
+                self.app.current_step = None
+                self.app.state_store.update_step("correction", variant=variant, batch_started=True)
+                statuses = {"submit_batches": "success", "batch_status": "detected",
+                            "fetch_batches": "success", "annotation": "success"}
+                for step_id, status in statuses.items():
+                    self.app.state_store.update_step(step_id, status=status)
+                self.app._populate_tree()
+                self.app.tree.selection_set("correction")
+                self.app.update()
+                with patch.object(self.app.runner, "start"), \
+                        patch.object(self.app, "_missing_requirements", return_value=[]):
+                    self.app._run_current_step()
+                self.assertEqual(self.app.active_step_id, "correction")
+                for step_id, status in statuses.items():
+                    expected = status if variant == "integrate" and step_id != "annotation" else "stale"
+                    self.assertEqual(self.app.state_store.step(step_id)["status"], expected)
+        self.app.active_step_id = None
+
+    def test_unsuccessful_batch_retrieval_stays_on_retrieval(self):
+        self.app.state_store.update_step("correction", variant="batch", last_run_variant="batch")
+        self.app._populate_tree()
+        self.app.tree.selection_set("fetch_batches")
+        self.app.update()
+        for code, interrupted in ((4, False), (1, False), (0, True)):
+            with self.subTest(code=code, interrupted=interrupted):
+                self.app.active_step_id = "fetch_batches"
+                self.app._finish_process(code, interrupted)
+                self.app.update()
+                self.assertEqual(self.app.current_step.id, "fetch_batches")
+                self.assertEqual(self.app.state_store.step("correction")["variant"], "batch")
+
     def test_batch_watch_buttons_use_loaded_evaluation_and_stop_on_reload(self):
         self.app.state_store.update_step("correction", variant="batch", last_run_variant="batch")
         self.app._populate_tree()
@@ -222,19 +272,24 @@ class GuiConvenienceTests(unittest.TestCase):
             self.app._move_selection_from("correction", 1)
         select.assert_called_once_with("manual_resolution")
 
-    def test_labels_and_correction_notify_once_on_success(self):
-        for step_id in ("labels", "correction"):
+    def test_long_running_steps_notify_once_on_success(self):
+        expected_text = {
+            "labels": "labels",
+            "correction": "correction",
+            "annotation": "copies annotées",
+            "read_annotations": "annotations manuscrites",
+        }
+        for step_id, completion_text in expected_text.items():
             with self.subTest(step=step_id), patch("copienator_gui.app.notify_desktop") as notify:
                 self.app.active_step_id = step_id
                 self.app._finish_process(0, False)
                 self.app._finish_process(0, False)
                 notify.assert_called_once()
                 self.assertIn(self.evaluation.name, notify.call_args.args[1])
-                self.assertIn("labels" if step_id == "labels" else "correction",
-                              notify.call_args.args[1])
+                self.assertIn(completion_text, notify.call_args.args[1])
 
     def test_incomplete_failed_and_interrupted_steps_notify_the_outcome_once(self):
-        for step_id in ("labels", "correction"):
+        for step_id in ("labels", "correction", "annotation", "read_annotations"):
             for code, interrupted, outcome in (
                 (4, False, "partielle"), (1, False, "échouée"),
                 (0, True, "interrompue"), (130, False, "interrompue"),

tests/test_gui_core.py

@@ -19,6 +19,7 @@ from PIL import Image
 from pypdf import PdfReader, PdfWriter
 
 from copienator import (
+    CliError,
     EvaluationWorkspace,
     WorkspaceNotFoundError,
     WorkspaceValidationError,
@@ -2058,7 +2059,23 @@ class StandardCliTests(unittest.TestCase):
             self.assertEqual(groups, [["Ex 1"], ["Ex 2"]])
             editor.assert_not_called()
 
-    def test_grouped_batching_does_not_split_one_student(self) -> None:
+    def test_grouped_batching_looks_ahead_to_keep_below_the_hard_limit(self) -> None:
+        module = self.modules["annotating_by_label"]
+        large = Image.new("RGB", (10, 80), "white")
+        small = Image.new("RGB", (10, 15), "white")
+        rendered = [
+            ("01", "Ex 1", large, 0, []),
+            ("02", "Ex 1", small, 0, []),
+            ("02", "Ex 2", small, 0, []),
+        ]
+        with patch.object(module, "MAX_HEIGHT_PX", 100):
+            batches = module.split_batches(rendered)
+        self.assertEqual([len(batch) for batch in batches], [1, 2])
+        self.assertTrue(
+            all(sum(item[2].height for item in batch) <= 100 for batch in batches)
+        )
+
+    def test_grouped_batching_never_splits_one_student(self) -> None:
         module = self.modules["annotating_by_label"]
         image = Image.new("RGB", (10, 60), "white")
         rendered = [
@@ -2066,9 +2083,16 @@ class StandardCliTests(unittest.TestCase):
             ("01", "Ex 2", image, 0, []),
             ("02", "Ex 1", image, 0, []),
         ]
-        with patch.object(module, "MAX_HEIGHT_PX", 100):
-            batches = module.split_batches(rendered)
-        self.assertEqual([len(batch) for batch in batches], [2, 1])
+        with patch.object(module, "MAX_HEIGHT_PX", 100), self.assertRaises(
+            CliError
+        ) as raised:
+            module.split_batches(rendered)
+        self.assertIn("Copie01", str(raised.exception))
+        self.assertIn("cannot be split", str(raised.exception))
+
+    def test_grouped_annotation_height_limit_is_32000_pixels(self) -> None:
+        module = self.modules["annotating_by_label"]
+        self.assertEqual(module.MAX_HEIGHT_PX, 32000)
 
 
 class WorkflowTests(unittest.TestCase):

tests/test_gui_manual_resolution.py

@@ -38,13 +38,31 @@ class ManualResolutionPanelTests(unittest.TestCase):
         self.assertEqual(panel.text.get("1.0", "end-1c"), content)
         self.assertEqual(len(panel.pdf_buttons), 4)
         self.assertEqual([button.cget("text") for button in panel.pdf_buttons],
-                         ["PDF source", "PDF cible"] * 2)
+                         ["Source : Ex 1", "Cible : Ex 2",
+                          "Source : Ex 3", "Cible : Ex 4"])
         with patch("copienator_gui.manual_resolution.open_path") as opened:
             panel.editor_button.invoke()
             for button in panel.pdf_buttons:
                 button.invoke()
         self.assertEqual(opened.call_args_list, [call(self.path), *map(call, paths)])
 
+    def test_reload_updates_the_displayed_target_and_its_pdf_action(self):
+        self.path.write_text("Copie01 A -> B|\n", encoding="utf-8")
+        copies = self.evaluation / "Copies" / "Copie01"
+        copies.mkdir(parents=True)
+        (copies / "A.pdf").touch()
+        (copies / "C.pdf").touch()
+        panel = self.panel()
+
+        self.path.write_text("Copie01 A -> C|\n", encoding="utf-8")
+        panel.reload_button.invoke()
+
+        self.assertEqual(panel.text.get("1.0", "end-1c"), "Copie01 A -> C|\n")
+        self.assertEqual(panel.pdf_buttons[1].cget("text"), "Cible : C")
+        with patch("copienator_gui.manual_resolution.open_path") as opened:
+            panel.pdf_buttons[1].invoke()
+        opened.assert_called_once_with(copies / "C.pdf")
+
     def test_reload_keeps_invalid_lines_visible_and_removes_stale_actions(self):
         self.path.write_text("Copie01 A -> B|\n", encoding="utf-8")
         panel = self.panel()

tests/test_manual_resolution_state.py

@@ -6,11 +6,40 @@ from unittest.mock import Mock, patch
 
 import pymupdf
 
-from copienator import EvaluationWorkspace, atomic_write_json, read_json
+from copienator import CliError, EvaluationWorkspace, atomic_write_json, read_json
 from copienator.commands import correction, resolve_manual
+from copienator_gui.app import has_manual_conflicts
 
 
 class ManualResolutionStateTests(unittest.TestCase):
+    def test_merge_into_missing_target_uses_source_and_schedules_redo(self):
+        for operator in ("->", "x>"):
+            for target in ("B|", "|B"):
+                with self.subTest(operator=operator, target=target), tempfile.TemporaryDirectory() as temporary:
+                    workspace = self.fixture(Path(temporary), operator)
+                    copies = workspace.copies_dir / "Copie01"
+                    (copies / "B.pdf").unlink()
+                    workspace.manual_resolutions_file.write_text(f"Copie01 A {operator} {target}\n")
+                    resolve_manual.resolve_manual(workspace)
+                    with pymupdf.open(copies / "B_new.pdf") as document:
+                        self.assertEqual(len(document), 1)
+                        self.assertIn("A", document[0].get_text())
+                    self.assertEqual((copies / "A_old.pdf").exists(), operator == "x>")
+                    self.assertEqual((copies / "A.pdf").exists(), operator == "->")
+                    self.assertFalse((copies / "B_old.pdf").exists())
+                    self.assertEqual(read_json(workspace.refaire_file), [["Copie01", ["B"]]])
+                    self.assertNotIn("delayed", read_json(workspace.correction_file)["A"][0][0]["result"])
+
+    def test_merge_still_requires_source_without_changing_state(self):
+        with tempfile.TemporaryDirectory() as temporary:
+            workspace = self.fixture(Path(temporary), "x>")
+            (workspace.copies_dir / "Copie01" / "A.pdf").unlink()
+            before = workspace.correction_file.read_bytes()
+            with self.assertRaises(CliError):
+                resolve_manual.resolve_manual(workspace)
+            self.assertEqual(workspace.correction_file.read_bytes(), before)
+            self.assertTrue(workspace.manual_resolutions_file.exists())
+
     def test_acknowledgement_clears_only_this_target_and_copy(self):
         for error in ("wrg-lbl:B?", "wrg-lbl:B?delayed", "wrg-lbl:B?exists", "al:(->)B?(->)C?", "al:(delayed)B"):
             with self.subTest(error=error):
@@ -63,3 +92,75 @@ class ManualResolutionStateTests(unittest.TestCase):
                 self.assertEqual(correction.run_configured(args), 0)
             self.assertFalse(workspace.manual_resolutions_file.exists())
             self.assertNotIn("delayed", correction.results["A"][0][0]["result"])
+
+    def test_deleted_conflict_stays_resolved_when_other_resolution_generates_refaire(self):
+        for kind in ("wrong-label", "add-label"):
+            with self.subTest(kind=kind), tempfile.TemporaryDirectory() as temporary:
+                workspace = self.fixture(Path(temporary), "x>")
+                other = workspace.copies_dir / "Copie02"
+                other.mkdir()
+                for label in ("A", "B"):
+                    (other / f"{label}.pdf").write_bytes(
+                        (workspace.copies_dir / "Copie01" / f"{label}.pdf").read_bytes())
+                before = {path.name: path.read_bytes() for path in other.iterdir()}
+                results = read_json(workspace.correction_file)
+                results["A"][0].append({"id": "02", "result": {
+                    "error": "wrg-lbl:B?" if kind == "wrong-label" else "al:(->)B?",
+                    "delayed": [[kind, "B"]], "score": 3,
+                }})
+                atomic_write_json(workspace.correction_file, results)
+                # Only Copie01 remains in the edited file; Copie02 was deleted.
+                resolve_manual.resolve_manual(workspace)
+                self.assertEqual(read_json(workspace.refaire_file), [["Copie01", ["B"]]])
+                self.assertEqual(before, {path.name: path.read_bytes() for path in other.iterdir()})
+                saved = read_json(workspace.correction_file)["A"][0][1]["result"]
+                self.assertNotIn("delayed", saved)
+                self.assertEqual(saved["score"], 3)
+                self.assertNotIn("B?", saved["error"])
+                (workspace.groups_dir / "B").mkdir(parents=True)
+                args = correction.build_parser().parse_args([str(workspace.root), "--refaire"])
+                correction.configure_runtime(workspace, [], args, api_client=Mock())
+                with patch.object(correction.grouping, "get_pdf_height", return_value=400), \
+                        patch.object(correction.grouping, "create_jpg"), \
+                        patch.object(correction, "process_single_task", return_value=[]):
+                    self.assertEqual(correction.run_configured(args), 0)
+                self.assertFalse(workspace.manual_resolutions_file.exists())
+                self.assertNotIn("delayed", correction.results["A"][0][1]["result"])
+
+    def test_deleting_all_lines_can_be_applied_and_survives_resume(self):
+        for text in ("", "### Instructions\n", "### Copie01 A x> B|\n"):
+            with self.subTest(text=text), tempfile.TemporaryDirectory() as temporary:
+                workspace = self.fixture(Path(temporary), "x>")
+                source = workspace.copies_dir / "Copie01"
+                before = {path.name: path.read_bytes() for path in source.iterdir()}
+                workspace.manual_resolutions_file.write_text(text)
+                self.assertTrue(has_manual_conflicts(workspace.manual_resolutions_file))
+                resolve_manual.resolve_manual(workspace)
+                self.assertEqual(before, {path.name: path.read_bytes() for path in source.iterdir()})
+                self.assertFalse(has_manual_conflicts(workspace.manual_resolutions_file))
+                atomic_write_json(workspace.refaire_file, [])
+                args = correction.build_parser().parse_args([str(workspace.root), "--refaire"])
+                correction.configure_runtime(workspace, [], args, api_client=Mock())
+                self.assertEqual(correction.run_configured(args), 0)
+                self.assertFalse(workspace.manual_resolutions_file.exists())
+
+    def test_invalid_remaining_instruction_does_not_commit_deletions(self):
+        with tempfile.TemporaryDirectory() as temporary:
+            workspace = self.fixture(Path(temporary), "x>")
+            before = workspace.correction_file.read_bytes()
+            workspace.manual_resolutions_file.write_text("invalid instruction\n")
+            with self.assertRaises(CliError):
+                resolve_manual.resolve_manual(workspace)
+            self.assertEqual(workspace.correction_file.read_bytes(), before)
+
+    def test_generated_first_comment_documents_deleting_lines(self):
+        with tempfile.TemporaryDirectory() as temporary:
+            workspace = self.fixture(Path(temporary), "x>")
+            atomic_write_json(workspace.refaire_file, [])
+            args = correction.build_parser().parse_args([str(workspace.root), "--refaire"])
+            correction.configure_runtime(workspace, [], args, api_client=Mock())
+            self.assertEqual(correction.run_configured(args), 0)
+            first_line = workspace.manual_resolutions_file.read_text().splitlines()[0]
+            self.assertTrue(first_line.startswith("###"))
+            self.assertIn("delete a line", first_line)
+            self.assertIn("resolve-manual", first_line)