Add per label remark, post correction

tests/test_custom_remarks.py

@@ -0,0 +1,85 @@
+from copy import deepcopy
+from pathlib import Path
+import tempfile
+import unittest
+from unittest.mock import patch
+
+from PIL import Image, ImageDraw
+
+from copienator import EvaluationWorkspace, ExitCode, atomic_write_json
+from copienator.custom_remarks import apply_custom_remarks, set_remark, remarks_for_copy
+from copienator.commands import annotating, reading_annotations as standard
+from copienator.commands import reading_grouped_annotations as grouped
+
+
+class CustomRemarkTests(unittest.TestCase):
+    def test_storage_preserves_other_students_and_feedback_is_idempotent(self):
+        with tempfile.TemporaryDirectory() as temporary:
+            root = Path(temporary)
+            set_remark(root, "01", "Ex 1", "First")
+            set_remark(root, "2", "Ex 1", "Other student")
+            set_remark(root, "1", "Ex 2", "Other label")
+            labels = {"Ex 1": {"result": {"feedback": [{"text": "Gemini"}]}}}
+            apply_custom_remarks(root, "1", labels)
+            apply_custom_remarks(root, "01", labels)
+            self.assertEqual([f["text"] for f in labels["Ex 1"]["result"]["feedback"]], ["Gemini", "First"])
+            set_remark(root, "01", "Ex 1", " ")
+            apply_custom_remarks(root, "1", labels)
+            self.assertEqual(labels["Ex 1"]["result"]["feedback"], [{"text": "Gemini"}])
+            self.assertEqual(remarks_for_copy(root, "1"), {"Ex 2": "Other label"})
+            self.assertEqual(remarks_for_copy(root, "02"), {"Ex 1": "Other student"})
+
+    def test_readers_render_remark_after_checkbox_deletion_and_preserve_ink(self):
+        for mode in ("Bnot", "BGnot"):
+            with self.subTest(mode=mode), tempfile.TemporaryDirectory() as temporary:
+                root = Path(temporary)
+                workspace = EvaluationWorkspace(root)
+                output = root / mode / "Copie01"
+                output.mkdir(parents=True)
+                answer = root / "answer.pdf"
+                answer.touch()
+                atomic_write_json(output / "bnote.json", {"images": [{"label": "Ex 1"}]})
+                atomic_write_json(output / "score.json", {"Ex 1": "4"})
+                original = {"01": {"Ex 1": {"pdf_path": answer, "coordinates": (0, 0),
+                    "result": {"score": 4, "feedback": [{"text": "Gemini"}]}}}}
+                set_remark(root, "1", "Ex 1", "Teacher")
+                ink = Image.new("RGBA", (1500, 110), (0, 0, 0, 0))
+                draw = ImageDraw.Draw(ink)
+                draw.rectangle((100, 1, 120, 8), fill="red")
+                draw.rectangle((100, 30, 120, 50), fill="blue")
+                real_compose = annotating.compose_label_image
+                def compose(*args, **kwargs):
+                    return real_compose(*args, **kwargs,
+                        render_fn=lambda text, *a, **k: Image.new("RGB", (1500, 20), "green"))
+                with patch.object(annotating, "make_base_image", return_value=(Image.new("RGB", (1500, 100), "white"), 0, 0)), \
+                     patch.object(annotating, "render_score_text", return_value=Image.new("RGB", (1500, 10), "white")), \
+                     patch.object(annotating, "compose_label_image", side_effect=compose), \
+                     patch.object(grouped, "get_extra_pdfs_as_images", return_value=[]):
+                    def run(with_ink=True):
+                        data = deepcopy(original)
+                        actions = [{"label": "Ex 1", "type": "clear_all"}]
+                        if mode == "Bnot":
+                            # The tablet header was ten pixels tall.
+                            atomic_write_json(output / "bnote.json", {"images": [
+                                {"label": "Ex 1", "header_height": 10}]})
+                            return standard.apply_actions_and_regenerate(workspace, data, "01", actions,
+                                None, ["Ex 1"], update_score=True, label_notes={"Ex 1": ink} if with_ink else {})
+                        status, _ = grouped.apply_actions_and_regenerate_grouped(workspace, data, "01",
+                            actions, {"Ex 1": {"img": ink, "old_header_h": 10}} if with_ink else {}, ["Ex 1"], update_score=True)
+                        return status
+                    self.assertEqual(run(), ExitCode.SUCCESS)
+                    self.assertTrue((output / "Concat_F.pdf").is_file())
+                    with Image.open(output / "Ex 1.jpg") as image:
+                        self.assertEqual(image.height, 130)
+                        self.assertGreater(image.getpixel((110, 4))[0], 180)
+                        self.assertGreater(image.getpixel((110, 65))[2], 180)
+                    # Even with no handwritten ink, a personal remark includes a perfect answer.
+                    original["01"]["Ex 1"]["result"]["feedback"] = []
+                    self.assertEqual(run(with_ink=False), ExitCode.SUCCESS)
+                    self.assertTrue((output / "Concat_F.pdf").is_file())
+                    set_remark(root, "1", "Ex 1", "")
+                    self.assertEqual(run(), ExitCode.SUCCESS)
+                    with Image.open(output / "Ex 1.jpg") as image:
+                        self.assertEqual(image.height, 110)
+                    self.assertEqual(run(with_ink=False), ExitCode.SUCCESS)
+                    self.assertFalse((output / "Concat_F.pdf").exists())

tests/test_gui_scores.py

@@ -5,6 +5,7 @@ from pathlib import Path
 from unittest.mock import patch
 
 from copienator import read_json
+from copienator.custom_remarks import remarks_for_copy
 from copienator_gui.app import CopienatorApp
 from copienator_gui.scores import score_path_for_id, set_score
 from copienator_gui.workflow import build_workflow
@@ -111,3 +112,39 @@ class ScoreEditorGuiTests(ScoreEditorTests):
         self.app._skip_step()
         self.assertEqual(self.app.state_store.step("edit_scores")["status"], "skipped")
         self.assertEqual(self.app.state_store.step("final_score")["status"], "success")
+
+    def test_remark_saves_independently_reloads_and_clears(self):
+        self.panel.id_var.set("1")
+        self.panel.label_var.set("Ex 1")
+        self.panel.score_var.set("bad unsaved score")
+        self.panel.remark_entry.insert("1.0", "Justifier.\nDétailler le calcul.")
+        self.panel.remark_button.invoke()
+        self.assertEqual(read_json(self.path)["Ex 1"], "2")
+        self.assertEqual(remarks_for_copy(self.evaluation, "01")["Ex 1"], "Justifier.\nDétailler le calcul.")
+        self.assertEqual(self.app.state_store.step("read_annotations")["status"], "stale")
+        self.panel.label_var.set("Ex 2")
+        self.assertEqual(self.panel.remark_entry.get("1.0", "end-1c"), "")
+        self.panel.label_var.set("Ex 1")
+        self.assertIn("Justifier.", self.panel.remark_entry.get("1.0", "end-1c"))
+        self.panel.remark_entry.delete("1.0", "end")
+        self.panel.remark_button.invoke()
+        self.assertEqual(remarks_for_copy(self.evaluation, "1"), {})
+        self.panel.set_busy(True)
+        self.assertEqual(str(self.panel.remark_button.cget("state")), "disabled")
+
+    def test_completing_editor_selects_regeneration_and_preserves_options(self):
+        self.app.state_store.update_step("read_annotations", status="success", variant="grouped",
+            values={"update_score": False, "return_answers_solution": False, "annotation_dir": "BGnot"})
+        with patch.object(self.app.runner, "start") as start:
+            self.app.run_button.invoke()
+            self.app.update()
+        start.assert_not_called()
+        self.assertEqual(self.app.state_store.step("edit_scores")["status"], "success")
+        self.assertEqual(self.app.current_step.id, "read_annotations")
+        self.assertEqual(self.app.tree.selection(), ("read_annotations",))
+        self.assertEqual(self.app.tree.item("read_annotations", "values"), ("À revalider",))
+        self.assertTrue(self.app.arg_vars["update_score"].get())
+        self.assertEqual(self.app.variant_var.get(), "grouped")
+        self.assertFalse(self.app.arg_vars["return_answers_solution"].get())
+        self.assertTrue(self.app.state_store.step("read_annotations")["values"]["update_score"])
+        self.assertIn("--update-score", self.app._make_command())

tests/test_incremental_annotation_reads.py

@@ -11,6 +11,7 @@ from PIL import Image
 from copienator import EvaluationWorkspace, ExitCode, atomic_write_json, configuration
 from copienator.annotation_data import AnnotationLoadResult
 from copienator.annotation_read_cache import STATE_FILE
+from copienator.custom_remarks import set_remark
 from copienator.commands import annotating, reading_annotations as standard
 from copienator.commands import reading_grouped_annotations as grouped
 
@@ -89,6 +90,19 @@ class IncrementalAnnotationReadsTests(unittest.TestCase):
                 self.assertEqual(scan.call_args.args[1], {"01"})
             run(0)
 
+            # Remarks are per-copy inputs, including their removal.
+            set_remark(root, "1", "Ex 1", "Justifier cette étape.")
+            run(1)
+            self.assertEqual(compose.call_args.args[2]["feedback"][-1]["text"], "Justifier cette étape.")
+            run(0)
+            set_remark(root, "01", "Ex 1", "Détailler le calcul.")
+            run(1)
+            self.assertEqual(len(compose.call_args.args[2]["feedback"]), 1)
+            set_remark(root, "1", "Ex 1", "")
+            run(1)
+            self.assertEqual(compose.call_args.args[2]["feedback"], [])
+            run(0)
+
             # Per-copy sources and correction data invalidate only that copy.
             (root / "Copies" / "01.pdf").write_bytes(b"replaced answer PDF")
             run(1)

tests/test_return_outputs.py

@@ -7,7 +7,7 @@ import unittest
 from pathlib import Path
 from unittest.mock import patch
 
-from copienator import EvaluationWorkspace, atomic_write_json, configuration
+from copienator import ExitCode, EvaluationWorkspace, atomic_write_json, configuration
 from copienator.commands import clean, giving_names
 
 
@@ -40,6 +40,26 @@ class ReturnOutputTests(unittest.TestCase):
         ), contextlib.redirect_stdout(io.StringIO()):
             self.assertEqual(giving_names.run(self.workspace, annotation_dir="BGnot"), 0)
 
+    def test_regeneration_refreshes_copied_named_files_and_removes_obsolete_pdf(self):
+        self.prepare()
+        for suffix in (".jpg", ".pdf"):
+            target = self.destination / ("Student" + suffix)
+            target.unlink()
+            target.write_bytes(b"old copied output")
+        (self.destination / "custom.pdf").write_bytes(b"keep")
+        with patch.multiple(configuration, RETURN_JPEG_ENABLED=True, RETURN_PDF_ENABLED=True):
+            status = giving_names.update_named_return_answers(
+                self.workspace, "BGnot", copy_ids={"01"}, refresh_named=True)
+            self.assertEqual(status, ExitCode.SUCCESS)
+            self.assertEqual((self.destination / "Student.pdf").read_bytes(), b"pdf")
+            self.assertEqual((self.destination / "Student.jpg").read_bytes(), b"jpeg")
+            (self.source / "Concat_F.pdf").unlink()
+            giving_names.update_named_return_answers(
+                self.workspace, "BGnot", copy_ids={"01"}, refresh_named=True)
+            self.assertFalse((self.destination / "Student.pdf").is_symlink())
+            self.assertFalse((self.destination / "Student.pdf").exists())
+            self.assertEqual((self.destination / "custom.pdf").read_bytes(), b"keep")
+
     def test_all_combinations_and_reenable_preserve_sources_and_scores(self):
         for jpeg, pdf in itertools.product((True, False), repeat=2):
             with self.subTest(jpeg=jpeg, pdf=pdf):