Initial support for user general remarks

tests/test_annotation_modifications.py

@@ -76,6 +76,21 @@ class ModificationTests(unittest.TestCase):
         self.assertTrue(all(abs(a-b) < 12 for a, b in zip(image.getpixel(xy), expected)),
                         (image.getpixel(xy), expected))
 
+    def test_general_remarks_survive_repeated_body_edits(self):
+        from copienator.general_remarks import IMAGE_FILENAME
+        for mode in ("Bnot", "BGnot"):
+            with self.subTest(mode=mode):
+                self.mode = mode
+                self.seed()
+                image = Image.new("RGB", (120, 60), "magenta")
+                image.save(self.output / IMAGE_FILENAME)
+                original = (self.output / IMAGE_FILENAME).read_bytes()
+                for _ in range(2):
+                    self.apply("Copie43 Ex 1 c{40}2> |Ex 2")
+                    self.assertEqual((self.output / IMAGE_FILENAME).read_bytes(), original)
+                    self.assert_color(self.image("Concat"), (50, 30), "magenta")
+                    self.assertTrue((self.output / "Concat_F.pdf").is_file())
+
     def test_cut_uses_body_height_preserves_own_headers_and_scores(self):
         self.apply("Copie43 Ex 1 c{40}2> |Ex 2")
         source, target = self.image("Ex 1"), self.image("Ex 2")

tests/test_general_remarks.py

@@ -0,0 +1,80 @@
+from copy import deepcopy
+from pathlib import Path
+import tempfile
+import unittest
+from unittest.mock import patch
+
+from PIL import Image
+from pdf2image import convert_from_path
+
+from copienator import EvaluationWorkspace, ExitCode, atomic_write_json
+from copienator import general_remarks as remarks
+from copienator.commands import annotating, reading_annotations as standard
+from copienator.commands import reading_grouped_annotations as grouped
+from copienator.return_answers import publish_answer_returns
+
+
+class GeneralRemarkTests(unittest.TestCase):
+    def test_order_normalization_and_deletion_preserve_other_copies(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            root = Path(tmp)
+            remarks.set_remarks(root, "01", [" First ", "Second"])
+            remarks.set_remarks(root, "2", ["Other"])
+            self.assertEqual(remarks.set_remarks(root, "1", ["Second", "First"]), ["First", "Second"])
+            self.assertEqual(remarks.remarks_for_copy(root, "01"), ["Second", "First"])
+            remarks.set_remarks(root, "1", [])
+            self.assertEqual(remarks.load_remarks(root), {"2": ["Other"]})
+
+    def test_latex_has_one_paragraph_per_remark_without_truncation(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            root = Path(tmp)
+            remarks.set_remarks(root, "1", [r"Justifier $x^2$.", "Soigner la rédaction."])
+            with patch.object(annotating, "render_real_latex_text", return_value=Image.new("RGB", (600, 150), "white")) as render:
+                image = remarks.render(root, "01", 800)
+            self.assertEqual(image.size, (800, 190))
+            self.assertIn(r"Justifier $x^2$.\par\medskip Soigner", render.call_args.args[0])
+            self.assertNotIn("max_lines", render.call_args.kwargs)
+
+    def test_readers_publish_header_even_for_perfect_answers_then_remove_it(self):
+        for mode in ("Bnot", "BGnot"):
+            with self.subTest(mode=mode), tempfile.TemporaryDirectory() as tmp:
+                root = Path(tmp)
+                workspace = EvaluationWorkspace(root)
+                output = root / mode / "Copie01"
+                output.mkdir(parents=True)
+                (root / "labels").write_text("Ex 1\n")
+                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"})
+                data = {"01": {"Ex 1": {"pdf_path": answer, "coordinates": (0, 0),
+                    "result": {"score": 4, "feedback": []}}}}
+                remarks.set_remarks(root, "1", ["First", "Second"])
+                def run():
+                    if mode == "Bnot":
+                        return standard.apply_actions_and_regenerate(workspace, deepcopy(data), "01", [], None, ["Ex 1"], update_score=True)
+                    return grouped.apply_actions_and_regenerate_grouped(workspace, deepcopy(data), "01", [], {}, ["Ex 1"], update_score=True)[0]
+                with patch.object(annotating, "make_base_image", return_value=(Image.new("RGB", (800, 100), "blue"), 0, 0)), \
+                     patch.object(annotating, "compose_label_image", return_value=(Image.new("RGB", (800, 100), "blue"), 10)), \
+                     patch.object(annotating, "render_real_latex_text", return_value=Image.new("RGB", (760, 60), "red")), \
+                     patch.object(grouped, "get_extra_pdfs_as_images", return_value=[]), \
+                     patch("copienator.configuration.RETURN_ANSWERS_ENABLED", True), \
+                     patch("copienator.return_answers.load_return_answer_options", return_value={"context": False, "question": False, "solution": False}):
+                    self.assertEqual(run(), ExitCode.SUCCESS)
+                    with Image.open(output / "Concat.jpg") as image:
+                        self.assertEqual(image.height, 200)
+                        self.assertGreater(image.getpixel((40, 40))[0], 230)
+                        self.assertGreater(image.getpixel((40, 150))[2], 230)
+                    pages = convert_from_path(output / "Concat_F.pdf", dpi=100)
+                    self.assertGreater(pages[0].getpixel((60, 60))[0], 230)
+                    destination = root / "A Rendre" / "Student (01)"
+                    publish_answer_returns(root, output, destination)
+                    self.assertTrue((destination / "answers" / remarks.IMAGE_FILENAME).is_file())
+                    remarks.set_remarks(root, "1", [])
+                    self.assertEqual(run(), ExitCode.SUCCESS)
+                    publish_answer_returns(root, output, destination)
+                    self.assertFalse((output / remarks.IMAGE_FILENAME).exists())
+                    self.assertFalse((output / "Concat_F.pdf").exists())
+                    self.assertFalse((destination / "answers" / remarks.IMAGE_FILENAME).exists())
+                    with Image.open(output / "Concat.jpg") as image:
+                        self.assertEqual(image.height, 100)

tests/test_gui_scores.py

@@ -132,6 +132,32 @@ class ScoreEditorGuiTests(ScoreEditorTests):
         self.panel.set_busy(True)
         self.assertEqual(str(self.panel.remark_button.cget("state")), "disabled")
 
+    def test_general_remarks_edits_invalidate_generation_and_preserve_options(self):
+        from copienator.general_remarks import remarks_for_copy as general_for_copy
+        self.app.tree.selection_set("general_remarks")
+        self.app.update()
+        panel = self.app.general_remarks_panel
+        panel.id_var.set("01")
+        panel.editor.insert("1.0", "First")
+        self.app.state_store.update_step("read_annotations", status="success",
+            values={"update_score": False, "return_answers_solution": False})
+        panel.save()
+        entry = self.app.state_store.step("read_annotations")
+        self.assertEqual(entry["status"], "stale")
+        self.assertTrue(entry["values"]["update_score"])
+        self.assertFalse(entry["values"]["return_answers_solution"])
+        panel.new()
+        panel.editor.insert("1.0", "Second")
+        panel.save()
+        panel.move(-1)
+        self.assertEqual(general_for_copy(self.evaluation, "1"), ["Second", "First"])
+        panel.delete()
+        panel.listbox.selection_set(0)
+        panel.delete()
+        self.assertEqual(general_for_copy(self.evaluation, "1"), [])
+        panel.id_var.set("2")
+        self.assertEqual(str(panel.editor.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"})
@@ -140,6 +166,9 @@ class ScoreEditorGuiTests(ScoreEditorTests):
             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, "general_remarks")
+        self.app.run_button.invoke()
+        self.app.update()
         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",))

tests/test_incremental_annotation_reads.py

@@ -12,6 +12,7 @@ from copienator import EvaluationWorkspace, ExitCode, atomic_write_json, configu
 from copienator.annotation_data import AnnotationLoadResult
 from copienator.annotation_read_cache import STATE_FILE
 from copienator.custom_remarks import set_remark
+from copienator.general_remarks import set_remarks, IMAGE_FILENAME
 from copienator.commands import annotating, reading_annotations as standard
 from copienator.commands import reading_grouped_annotations as grouped
 
@@ -103,6 +104,19 @@ class IncrementalAnnotationReadsTests(unittest.TestCase):
             self.assertEqual(compose.call_args.args[2]["feedback"], [])
             run(0)
 
+            with patch.object(annotating, "render_real_latex_text",
+                              return_value=Image.new("RGB", (100, 30), "white")):
+                set_remarks(root, "1", ["First", "Second"])
+                run(1)
+                run(0)
+                set_remarks(root, "1", ["Second", "First"])
+                run(1)
+                (root / mode / "Copie01" / IMAGE_FILENAME).unlink()
+                run(1)
+                set_remarks(root, "1", [])
+                run(1)
+                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_left_margin.py

@@ -118,6 +118,81 @@ class LeftMarginTests(unittest.TestCase):
         mask[300:960] = False
         self.assertIsNone(left_margin.body_edge(mask)['edge_mm'])
 
+    def test_bands_starting_beyond_midpoint_are_empty_for_both_windows(self):
+        mask = np.zeros((1169, 827), bool)
+        for y in range(370, 1050, 35):
+            mask[y:y + 5, 450:550] = True
+        result = left_margin.body_edge(mask)
+        self.assertIsNone(result['edge_mm'])
+        self.assertEqual(result['band_starts_mm'], [])
+        self.assertEqual(result['label_band_starts_mm'], [])
+        mask[400:410, 100:200] = True
+        self.assertTrue(left_margin.body_edge(mask)['band_starts_mm'])
+
+    def test_label_window_keeps_lower_body_but_excludes_names_and_bottom_eight_percent(self):
+        mask = np.zeros((1169, 827), bool)
+        mask[100:300, 5:80] = True  # Name/header; exclude from both pools.
+        mask[1100:1150, 5:80] = True  # Footer; below 92%.
+        for y in range(370, 920, 35):
+            mask[y:y + 5, 170:270] = True
+        mask[1000:1010, 40:140] = True  # Label at 85.5%; new window only.
+        result = left_margin.body_edge(mask)
+        self.assertGreater(min(result['band_starts_mm']), 40)
+        self.assertAlmostEqual(min(result['label_band_starts_mm']), 10.16)
+
+    def test_fifth_percentile_detects_labels_while_page_decile_stays_at_body(self):
+        images = [Image.new('RGB', (1654, 200)) for _ in range(6)]
+        bands = [5.] * 7 + [42.] * 93
+        estimate = {'edge_mm': left_margin.trimmed_left_edge(bands),
+                    'bands': len(bands), 'band_starts_mm': bands,
+                    'label_band_starts_mm': bands}
+        with patch.object(left_margin, 'estimate_page', return_value=estimate):
+            result = left_margin.analyse_margins(images, 200, 1654)
+        group = result['groups'][0]
+        self.assertEqual(group['edge_mm'], 42)
+        self.assertEqual(group['body_left_mm'], 22)
+        self.assertEqual(group['label_edge_mm'], 5)
+        self.assertTrue(group['labels_in_margin'])
+        self.assertEqual(result['starts_px'], [0] * 6)
+
+    def test_unsampled_coherent_page_can_move_the_whole_group_left(self):
+        images = [Image.new('RGB', (1654, 200)) for _ in range(6)]
+        def estimate(image, _dpi):
+            labels = [5.] * 20 if image is images[2] else []
+            return {'edge_mm': 42, 'bands': 12, 'band_starts_mm': [42.] * 12,
+                    'label_band_starts_mm': labels + [42.] * 12}
+        with patch.object(left_margin, 'estimate_page', side_effect=estimate):
+            result = left_margin.analyse_margins(images, 200, 1654)
+        group = result['groups'][0]
+        self.assertNotIn(3, [p['page'] for p in group['samples']])
+        self.assertIn(3, group['coherent_pages'])
+        self.assertEqual(result['starts_px'], [0] * 6)
+
+    def test_sparse_or_incoherent_pages_do_not_supply_label_evidence(self):
+        images = [Image.new('RGB', (1654, 200)) for _ in range(6)]
+        edges = [42, 42, None, 42, 65, 42]
+        def estimate(image, _dpi):
+            edge = edges[next(i for i, candidate in enumerate(images) if candidate is image)]
+            return {'edge_mm': edge, 'bands': 12, 'band_starts_mm': [edge or 42.] * 12,
+                    'label_band_starts_mm': [1.] * 20 if edge in (None, 65) else [42.] * 12}
+        with patch.object(left_margin, 'estimate_page', side_effect=estimate):
+            result = left_margin.analyse_margins(images, 200, 1654)
+        group = result['groups'][0]
+        self.assertEqual(group['coherent_pages'], [1, 2, 4, 6])
+        self.assertFalse(group['labels_in_margin'])
+        self.assertGreater(result['starts_px'][0], 0)
+
+    def test_less_than_five_percent_noise_or_small_gap_does_not_trigger_labels(self):
+        for bands in ([1.] * 4 + [42.] * 96, [35.] * 7 + [42.] * 93):
+            with self.subTest(bands=bands[:8]):
+                images = [Image.new('RGB', (1654, 200)) for _ in range(2)]
+                estimate = {'edge_mm': 42, 'bands': 100, 'band_starts_mm': [42.] * 100,
+                            'label_band_starts_mm': bands}
+                with patch.object(left_margin, 'estimate_page', return_value=estimate):
+                    result = left_margin.analyse_margins(images, 200, 1654)
+                self.assertFalse(result['groups'][0]['labels_in_margin'])
+                self.assertGreater(result['starts_px'][0], 0)
+
     def test_real_mask_rejects_ruling_and_preserves_handwriting(self):
         image = Image.new('RGB', (827, 1169), 'white')
         draw = ImageDraw.Draw(image)

tests/test_margin_benchmark.py

@@ -0,0 +1,55 @@
+"""Check benchmark scoring and independent fixture annotations (no PDF render)."""
+import hashlib
+import importlib.util
+import json
+import unittest
+from pathlib import Path
+
+ROOT = Path(__file__).resolve().parents[1]
+SPEC = importlib.util.spec_from_file_location('check_margins', ROOT / 'cropping-test/check_margins.py')
+benchmark = importlib.util.module_from_spec(SPEC)
+SPEC.loader.exec_module(benchmark)
+
+
+class MarginBenchmarkTests(unittest.TestCase):
+    def test_label_must_fit_both_strip_edges_even_when_start_is_acceptable(self):
+        expected = {'start_mm_range': [0, 20], 'labels': [
+            {'box_px': [20, 100, 50, 140], 'label': 'left heading'},
+            {'box_px': [590, 200, 630, 240], 'label': 'right heading'},
+        ]}
+        result = benchmark.score_page({'left_px': 30, 'right_px': 620}, expected)
+        self.assertTrue(result['start_in_range'])
+        self.assertFalse(result['pass'])
+        self.assertEqual(len(result['missed_labels']), 2)
+        self.assertTrue(benchmark.score_page({'left_px': 20, 'right_px': 630}, expected)['pass'])
+
+    def test_unannotated_page_still_checks_reviewed_start_range(self):
+        expected = {'start_mm_range': [0, 3], 'labels': []}
+        self.assertFalse(benchmark.score_page({'left_px': 116, 'right_px': 722}, expected)['pass'])
+        self.assertTrue(benchmark.score_page({'left_px': 0, 'right_px': 606}, expected)['pass'])
+
+    def test_fixture_provenance_whole_copy_order_and_suggested_strip_containment(self):
+        manifest = json.loads((benchmark.ROOT / 'manifest.json').read_text())
+        self.assertEqual(manifest['render_dpi'], 200)
+        self.assertEqual({c['id'] for c in manifest['cases']},
+                         {f'DS02VA-Copie{n:02}' for n in (1, 2, 3, 5, 8, 10, 16, 24, 25, 32)})
+        for case in manifest['cases']:
+            with self.subTest(case=case['id']):
+                self.assertEqual(hashlib.sha256((benchmark.ROOT / case['source']).read_bytes()).hexdigest(),
+                                 case['sha256'])
+                self.assertEqual([p['page'] for p in case['pages']], list(range(1, case['page_count'] + 1)))
+                self.assertTrue(case['explanation'])
+                self.assertTrue(any(p['labels'] for p in case['pages']))
+                for expected, recorded in zip(case['pages'], case['recorded_schema']['page_crops'], strict=True):
+                    start = round(expected['suggested_start_mm'] * 200 / 25.4)
+                    crop = {'left_px': start, 'right_px': min(recorded['width_px'],
+                            start + recorded['right_px'] - recorded['left_px'])}
+                    self.assertTrue(benchmark.score_page(crop, expected)['pass'])
+                    for label in expected['labels']:
+                        x0, y0, x1, y1 = label['box_px']
+                        self.assertTrue(0 <= x0 < x1 <= recorded['width_px'])
+                        self.assertTrue(0 <= y0 < y1 <= recorded['height_px'])
+
+
+if __name__ == '__main__':
+    unittest.main()