From 2c9dc6564040badd35bea015da935263f9686d9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Miquel?= Date: Wed, 28 Oct 2026 13:51:59 +0100 Subject: [PATCH] Initial support for user general remarks --- copienator/annotation_modifications.py | 15 +- copienator/annotation_read_cache.py | 5 +- copienator/commands/reading_annotations.py | 12 +- .../commands/reading_grouped_annotations.py | 14 +- copienator/general_remarks.py | 68 +++++++++ copienator/left_margin.py | 55 ++++++- copienator/return_answers.py | 6 + copienator_gui/app.py | 35 ++++- copienator_gui/general_remarks.py | 137 ++++++++++++++++++ copienator_gui/workflow.py | 11 ++ tests/test_annotation_modifications.py | 15 ++ tests/test_general_remarks.py | 80 ++++++++++ tests/test_gui_scores.py | 29 ++++ tests/test_incremental_annotation_reads.py | 14 ++ tests/test_left_margin.py | 75 ++++++++++ tests/test_margin_benchmark.py | 55 +++++++ 16 files changed, 606 insertions(+), 20 deletions(-) create mode 100644 copienator/general_remarks.py create mode 100644 copienator_gui/general_remarks.py create mode 100644 tests/test_general_remarks.py create mode 100644 tests/test_margin_benchmark.py diff --git a/copienator/annotation_modifications.py b/copienator/annotation_modifications.py index 486e08a..dbbb01e 100644 --- a/copienator/annotation_modifications.py +++ b/copienator/annotation_modifications.py @@ -2,6 +2,9 @@ from __future__ import annotations import re +import shutil + +from copienator import general_remarks from copy import deepcopy from hashlib import sha256 from pathlib import Path @@ -154,7 +157,7 @@ def edited_bodies(directory: Path, meta: dict, instructions: list[ManualInstruct def output_removals(directory: Path) -> tuple[str, ...]: info = read_json(directory / "info.json") if (directory / "info.json").exists() else {} - return (STATE_FILE, "Concat.jpg", "Concat_F.jpg", "Concat_F.pdf", "touched.json", "answer_labels.json", + return (general_remarks.IMAGE_FILENAME, STATE_FILE, "Concat.jpg", "Concat_F.jpg", "Concat_F.pdf", "touched.json", "answer_labels.json", *(f"{label}.jpg" for label, entry in info.items() if Path(label).name == label)) @@ -181,7 +184,13 @@ def rebuild(root: Path, mode: str, directory: Path, staging: Path, meta: dict, label for label, entry in info.items() if entry["not_empty"] } all_labels = utils.read_all_labels(root) - images, groups, checked_images = [], [], [] + general = general_remarks.existing(directory) + images = [general] if general is not None else [] + groups = [[general]] if general is not None else [] + checked_images = [] + if general is not None and directory != staging: + shutil.copy2(directory / general_remarks.IMAGE_FILENAME, + staging / general_remarks.IMAGE_FILENAME) seen_contexts = set() order = sorted(bodies, key=utils.natural_key) for label in order: @@ -215,7 +224,7 @@ def rebuild(root: Path, mode: str, directory: Path, staging: Path, meta: dict, save_paginated_pdf(groups, staging / "Concat_F.pdf") else: stack([image for group in groups for image in group]).save(staging / "Concat_F.jpg") - save_checked_pdf(root, checked_images, all_labels, staging / "Concat_F.pdf") + save_checked_pdf(root, checked_images, all_labels, staging / "Concat_F.pdf", general=general) def stage_baseline(root: Path, mode: str, output: Path, staging: Path, diff --git a/copienator/annotation_read_cache.py b/copienator/annotation_read_cache.py index 5846d6e..8de4ed8 100644 --- a/copienator/annotation_read_cache.py +++ b/copienator/annotation_read_cache.py @@ -7,6 +7,7 @@ from copienator import atomic_write_json, configuration, read_json from copienator.annotation_documents import copy_documents from copienator.return_answers import load_return_answer_options from copienator.custom_remarks import load_remarks, copy_key +from copienator import general_remarks STATE_FILE = ".annotation-read-state.json" DOCUMENT_INPUTS = ("bnote.json", "checkboxes.json", "Reference.jpg", "Concat_annotated.pdf") @@ -40,6 +41,7 @@ class AnnotationReadCache: configuration.RETURN_JPEG_ENABLED, configuration.RETURN_PDF_ENABLED, configuration.LATEX_ANOT_BEFORE, configuration.LATEX_ANOT_AFTER) remarks = load_remarks(root) + general = general_remarks.load_remarks(root) for copy_id, labels in data.items(): directory = root / mode / f"Copie{copy_id}" paths = [directory / "score.json", directory / "bnote.json"] @@ -48,7 +50,7 @@ class AnnotationReadCache: for content in labels.values(): pdf = Path(content["pdf_path"]) paths.extend((pdf, pdf.with_suffix(".layout.json"))) - payload = json.dumps((2, common, labels, remarks.get(copy_key(copy_id), {}), _files_digest(paths)), + payload = json.dumps((3, common, labels, general.get(copy_key(copy_id), []), remarks.get(copy_key(copy_id), {}), _files_digest(paths)), sort_keys=True, default=str).encode() self.signatures[copy_id] = sha256(payload).hexdigest() @@ -69,6 +71,7 @@ class AnnotationReadCache: info = read_json(directory / "info.json") outputs.extend(directory / f"{label}.jpg" for label, entry in info.items() if entry["present"]) + outputs.extend(directory.glob(general_remarks.IMAGE_FILENAME)) outputs.extend(directory.glob("Concat*.*")) outputs.extend(directory.glob(".annotation-base-*.png")) outputs.extend(directory.glob("annotation_baseline.json")) diff --git a/copienator/commands/reading_annotations.py b/copienator/commands/reading_annotations.py index f682571..8afd16b 100644 --- a/copienator/commands/reading_annotations.py +++ b/copienator/commands/reading_annotations.py @@ -26,6 +26,7 @@ from copienator.annotation_documents import copy_documents from copienator.annotation_layout import legacy_layout from copienator.annotation_read_cache import AnnotationReadCache from copienator.custom_remarks import apply_custom_remarks +from copienator import general_remarks from copienator.annotation_modifications import ( BASELINE, output_removals, @@ -260,9 +261,16 @@ def apply_actions_and_regenerate( if incomplete: return ExitCode.PARTIAL + general = general_remarks.render(workspace.root, student_id, + max((im.width for im in dirty_images.values()), default=800)) + if general is not None: + concatenated.insert(0, general) + filtered.insert(0, general) concat_image = concatenate(concatenated) filtered_image = concatenate(filtered) with staged_files(output_dir, remove=(*output_removals(output_dir), BASELINE)) as staging: + if general is not None: + general.save(staging / general_remarks.IMAGE_FILENAME) for label, image in dirty_images.items(): image.save(staging / f"{label}.jpg") if not preserve_score_file: @@ -277,8 +285,8 @@ def apply_actions_and_regenerate( # Imported here because the grouped reader also uses this module. from copienator.commands.reading_grouped_annotations import save_checked_pdf save_checked_pdf( - workspace.root, list(zip(filtered_labels, filtered, strict=True)), - all_labels, staging / "Concat_F.pdf", + workspace.root, [(label, dirty_images[label]) for label in filtered_labels], + all_labels, staging / "Concat_F.pdf", general=general, ) stage_baseline(workspace.root, "Bnot", output_dir, staging, dirty_images, header_heights, build_answer_info(scores, labels_data, answer_labels), diff --git a/copienator/commands/reading_grouped_annotations.py b/copienator/commands/reading_grouped_annotations.py index 8d58844..5ed9dfd 100644 --- a/copienator/commands/reading_grouped_annotations.py +++ b/copienator/commands/reading_grouped_annotations.py @@ -29,6 +29,7 @@ from copienator.annotation_documents import copy_documents, tablet_documents from copienator.annotation_layout import legacy_layout from copienator.annotation_read_cache import AnnotationReadCache from copienator.custom_remarks import apply_custom_remarks +from copienator import general_remarks from copienator.annotation_modifications import ( BASELINE, baseline, @@ -150,9 +151,9 @@ def save_paginated_pdf( ) -def save_checked_pdf(root, labelled_images, all_labels, output_path): +def save_checked_pdf(root, labelled_images, all_labels, output_path, *, general=None): """Build the checked-mode return PDF with the same supplements as BGnot.""" - groups = [] + groups = [[general]] if general is not None else [] seen_contexts = set() for label, image in labelled_images: extras = get_extra_pdfs_as_images( @@ -410,10 +411,17 @@ def apply_actions_and_regenerate_grouped( filtered_groups.append([*extras, final_image]) touched[label] = annotation_dir == "BGnot" + general = general_remarks.render(workspace.root, student_id, + max((im.width for im in dirty_images.values()), default=800)) + if general is not None: + concat_images.insert(0, general) + filtered_groups.insert(0, [general]) concat_image = concatenate(concat_images) if incomplete: return ExitCode.PARTIAL, "\n".join(logs) with staged_files(output_dir, remove=(*output_removals(output_dir), BASELINE)) as staging: + if general is not None: + general.save(staging / general_remarks.IMAGE_FILENAME) if simple_layout is not None: simple_layout["replaced"] = sorted( set(simple_layout["replaced"]) | selected_labels @@ -440,7 +448,7 @@ def apply_actions_and_regenerate_grouped( save_checked_pdf( workspace.root, [(label, dirty_images[label]) for label in filtered_labels], - all_labels, staging / "Concat_F.pdf", + all_labels, staging / "Concat_F.pdf", general=general, ) stage_baseline(workspace.root, annotation_dir, output_dir, staging, dirty_images, header_heights, build_answer_info(scores, labels_data, answer_labels, touched), diff --git a/copienator/general_remarks.py b/copienator/general_remarks.py new file mode 100644 index 0000000..e84fc96 --- /dev/null +++ b/copienator/general_remarks.py @@ -0,0 +1,68 @@ +"""Ordered copy-level remarks and their generated LaTeX image.""" +from pathlib import Path + +from PIL import Image + +from copienator import atomic_update_json, read_json +from copienator.custom_remarks import copy_key + +FILENAME = "general_remarks.json" +IMAGE_FILENAME = "Remarques_generales.jpg" + + +def validate(data): + if not isinstance(data, dict) or any( + not isinstance(key, str) or copy_key(key) != key + or not isinstance(remarks, list) + or any(not isinstance(text, str) for text in remarks) + for key, remarks in data.items() + ): + raise ValueError(f"Format invalide dans {FILENAME}.") + + +def load_remarks(root: Path) -> dict[str, list[str]]: + data = read_json(root / FILENAME, default={}) + validate(data) + return data + + +def remarks_for_copy(root: Path, copy_id: str) -> list[str]: + return load_remarks(root).get(copy_key(copy_id), []) + + +def set_remarks(root: Path, copy_id: str, remarks: list[str]) -> list[str]: + key = copy_key(copy_id) + current = [text.strip() for text in remarks if text.strip()] + previous = [] + + def update(data): + nonlocal previous + validate(data) + previous = data.get(key, []) + if current: + data[key] = current + else: + data.pop(key, None) + + atomic_update_json(root / FILENAME, update, default_factory=dict) + return previous + + +def render(root: Path, copy_id: str, width: int) -> Image.Image | None: + remarks = remarks_for_copy(root, copy_id) + if not remarks: + return None + from copienator.commands.annotating import render_real_latex_text + text = r"\textbf{Remarques générales}\par\medskip " + r"\par\medskip ".join(remarks) + content = render_real_latex_text(text, max(100, width - 40)).convert("RGB") + image = Image.new("RGB", (max(width, content.width + 40), content.height + 40), "white") + image.paste(content, (20, 20)) + return image + + +def existing(directory: Path) -> Image.Image | None: + path = directory / IMAGE_FILENAME + if path.is_file(): + with Image.open(path) as image: + return image.convert("RGB") + return None diff --git a/copienator/left_margin.py b/copienator/left_margin.py index 410fc94..25282ea 100644 --- a/copienator/left_margin.py +++ b/copienator/left_margin.py @@ -13,6 +13,10 @@ WIDTH_TOLERANCE = .08 # Start below it with clearance, including on later pages with repeated names. BODY_TOP = .30 BODY_BOTTOM = .82 +LABEL_BOTTOM = .92 +LABEL_LEFT_FRACTION = .05 +COHERENT_MARGIN_TOLERANCE_MM = 10 +LABEL_MARGIN_GAP_MM = 10 DISCARD_LEFT_FRACTION = .10 STANDARD_MARGIN_MM = 40 MARGIN_CLEARANCE_MM = 20 @@ -47,21 +51,28 @@ def handwriting_mask(rgb: np.ndarray) -> np.ndarray: return keep[labels] & diagnostics['masks']['foreground'] -def body_edge(mask: np.ndarray) -> dict: - """Estimate a left edge from occupied 4 mm bands within 30–82% height.""" - height = mask.shape[0] +def occupied_band_starts(mask: np.ndarray, bottom_fraction: float) -> list[float]: + """Ignore empty bands and writing starting beyond the page midpoint.""" + height, width = mask.shape band_height = max(1, round(4 * ANALYSIS_DPI / 25.4)) - bottom = round(BODY_BOTTOM * height) + bottom = round(bottom_fraction * height) starts = [] for y in range(round(BODY_TOP * height), bottom, band_height): band = mask[y:min(y + band_height, bottom)] xs = np.flatnonzero(band.sum(axis=0) >= 2) - if band.sum() >= 40 and len(xs) >= 12: + if band.sum() >= 40 and len(xs) >= 12 and xs[0] <= width / 2: starts.append(float(xs[0]) * 25.4 / ANALYSIS_DPI) + return starts + + +def body_edge(mask: np.ndarray) -> dict: + """Keep the body decile; collect a wider 30–92% label-search window.""" + starts = occupied_band_starts(mask, BODY_BOTTOM) return { 'edge_mm': trimmed_left_edge(starts) if len(starts) >= 6 else None, 'bands': len(starts), 'band_starts_mm': starts, + 'label_band_starts_mm': occupied_band_starts(mask, LABEL_BOTTOM), } @@ -90,7 +101,7 @@ def format_groups(images: list[Image.Image], reference_width: float) -> list[lis def analyse_margins(images: list[Image.Image], render_dpi: int, reference_width: float) -> dict: - """Pool occupied bands and use the larger half-margin or 20 mm-clearance cut.""" + """Estimate body margins, then retain left labels supported across coherent pages.""" starts = [0] * len(images) groups = [] page_estimates = {} @@ -116,6 +127,24 @@ def analyse_margins(images: list[Image.Image], render_dpi: int, reference_width: status = 'near-edge' if edge < MIN_MARGIN_MM else 'margin' left_mm = (0.0 if edge < MIN_MARGIN_MM else max(edge / 2, edge - MARGIN_CLEARANCE_MM)) + # Use ALL coherent pages, not just the four body-margin samples. + # A sparse/low/outlying page cannot supply evidence for this format's + # established body margin. Its normal fallback behaviour remains below. + coherent = [index for index in indices + if edge is not None and page_estimate(index)['edge_mm'] is not None + and page_estimate(index)['edge_mm'] >= MIN_MARGIN_MM + and abs(page_estimate(index)['edge_mm'] - edge) <= COHERENT_MARGIN_TOLERANCE_MM] + label_starts = [start for index in coherent for start in + page_estimate(index).get('label_band_starts_mm', + page_estimate(index)['band_starts_mm'])] + label_edge = (round(sorted(label_starts)[int(len(label_starts) * LABEL_LEFT_FRACTION)], 1) + if label_starts else None) + labels_in_margin = (status == 'margin' and len(coherent) >= min(2, len(indices)) + and label_edge is not None and edge - label_edge >= LABEL_MARGIN_GAP_MM) + body_left_mm = left_mm + if labels_in_margin: + status = 'labels-in-margin' + left_mm = 0.0 # Secondary subpages can start in the middle of an answer. Preserve # their left edge even when the only visible writing is indented. if images[indices[0]].width < .8 * reference_width: @@ -129,6 +158,11 @@ def analyse_margins(images: list[Image.Image], render_dpi: int, reference_width: 'status': status, 'edge_mm': edge, 'left_mm': round(left_mm, 1), + 'body_left_mm': round(body_left_mm, 1), + 'labels_in_margin': labels_in_margin, + 'coherent_pages': [index + 1 for index in coherent], + 'label_edge_mm': label_edge, + 'label_bands': len(label_starts), 'bands': len(band_starts), 'discarded_bands': int(len(band_starts) * DISCARD_LEFT_FRACTION), 'samples': [{'page': index + 1, **item} for index, item in estimates.items()], @@ -154,9 +188,14 @@ def analyse_margins(images: list[Image.Image], render_dpi: int, reference_width: 'status': 'unknown' if edge is None else ('low' if edge < MIN_MARGIN_MM else 'normal')}) - return {'version': 8, 'pages': pages, 'starts_px': starts, 'groups': groups, + return {'version': 9, 'pages': pages, 'starts_px': starts, 'groups': groups, 'no_margin_from_page': no_margin_from_page, 'discard_left_fraction': DISCARD_LEFT_FRACTION, 'margin_clearance_mm': MARGIN_CLEARANCE_MM, 'cut_fraction_of_margin': .5, 'cut_rule': 'maximum', 'standard_margin_mm': STANDARD_MARGIN_MM, 'minimum_margin_mm': MIN_MARGIN_MM, - 'body_height_fraction': [BODY_TOP, BODY_BOTTOM]} + 'body_height_fraction': [BODY_TOP, BODY_BOTTOM], + 'label_height_fraction': [BODY_TOP, LABEL_BOTTOM], + 'label_left_fraction': LABEL_LEFT_FRACTION, + 'coherent_margin_tolerance_mm': COHERENT_MARGIN_TOLERANCE_MM, + 'label_margin_gap_mm': LABEL_MARGIN_GAP_MM, + 'maximum_band_start_width_fraction': .5} diff --git a/copienator/return_answers.py b/copienator/return_answers.py index bebaad5..e8a1104 100644 --- a/copienator/return_answers.py +++ b/copienator/return_answers.py @@ -1,6 +1,9 @@ from __future__ import annotations from pathlib import Path +import shutil + +from copienator import general_remarks from PIL import Image @@ -92,6 +95,9 @@ def publish_answer_returns( all_labels = utils.read_all_labels(root) with staged_directory(answers_dir) as staging: + general = source / general_remarks.IMAGE_FILENAME + if general.is_file(): + shutil.copy2(general, staging / general.name) for label in sorted(labels, key=utils.natural_key): paths = [] if options["context"]: diff --git a/copienator_gui/app.py b/copienator_gui/app.py index ec9434d..ce5f9d9 100644 --- a/copienator_gui/app.py +++ b/copienator_gui/app.py @@ -29,6 +29,7 @@ from .diagnostics import collect_diagnostics from .batch_monitor import BatchMonitor from .giving_names import GivingNamesPanel, find_name_issues from .scores import ScoreEditorPanel +from .general_remarks import GeneralRemarksPanel from .notifications import notify_desktop from .syncthing import SyncthingPanel from .manual_resolution import ManualResolutionPanel @@ -285,6 +286,7 @@ class CopienatorApp(tk.Tk): self._rendering = False self.refaire_panel: RefaireSelection | None = None self.giving_names_panel: GivingNamesPanel | None = None + self.general_remarks_panel = None self.score_editor_panel: ScoreEditorPanel | None = None self.complete_giving_names_button: ttk.Button | None = None self.label_groups_button: ttk.Button | None = None @@ -816,7 +818,7 @@ class CopienatorApp(tk.Tk): if not step: return self._rendering = True - if step.id in {"personal_read", "edit_scores"}: + if step.id in {"personal_read", "edit_scores", "general_remarks"}: self.command_box.grid_remove() else: self.command_box.grid() @@ -831,6 +833,7 @@ class CopienatorApp(tk.Tk): "modify_annotations", "giving_names", "edit_scores", + "general_remarks", "statement_persp", } self.console.configure(height=8 if expanded_form else 14) @@ -844,6 +847,7 @@ class CopienatorApp(tk.Tk): self.arg_vars.clear() self.refaire_panel = None self.giving_names_panel = None + self.general_remarks_panel = None self.score_editor_panel = None self.complete_giving_names_button = None self.label_groups_button = None @@ -1084,6 +1088,12 @@ class CopienatorApp(tk.Tk): ) row += 1 return row + if step.id == "general_remarks" and self.evaluation: + self.general_remarks_panel = GeneralRemarksPanel( + self.form, self.evaluation, self._general_remarks_changed + ) + self.general_remarks_panel.grid(row=row, column=0, columnspan=2, sticky="ew", pady=8) + row += 1 if step.id == "edit_scores" and self.evaluation: self.score_editor_panel = ScoreEditorPanel( self.form, self.evaluation, self._score_changed, self._remark_changed @@ -1644,7 +1654,7 @@ class CopienatorApp(tk.Tk): if (step.is_manual or step.id == "personal_create" or (step.id == "personal_read" and review_command is None)): self._mark_step("success") - if step.id == "edit_scores": + if step.id == "general_remarks": entry = self.state_store.step("read_annotations") values = dict(entry.get("values", {})) values["update_score"] = True @@ -1778,7 +1788,7 @@ class CopienatorApp(tk.Tk): return self._save_current_form() # Score edits already invalidate downstream steps when Set saves a change. - if self.current_step.id != "edit_scores": + if self.current_step.id not in {"edit_scores", "general_remarks"}: self._invalidate_after_step(self.current_step, self._current_variant().id) self.state_store.update_step(self.current_step.id, status=status) history: dict[str, object] = { @@ -1808,6 +1818,23 @@ class CopienatorApp(tk.Tk): self._populate_tree() self.info_var.set("Renommage validé — aucun fichier modifié.") + def _general_remarks_changed(self, copy_id, previous, current): + step = self.step_by_id["general_remarks"] + self._invalidate_after_step(step, "default") + self.state_store.update_step(step.id, status="detected") + entry = self.state_store.step("read_annotations") + values = dict(entry.get("values", {})) + values["update_score"] = True + self.state_store.update_step("read_annotations", status="stale", values=values) + self.state_store.add_history({ + "step": step.id, "copy_id": copy_id, "previous_remarks": previous, + "remarks": current, "manual": True, + }) + for item in self.steps: + if self.tree.exists(item.id): + status = self._step_status(item) + self.tree.item(item.id, values=(STATUS_LABELS.get(status, status),)) + def _remark_changed(self, copy_id: str, label: str, previous: str, current: str) -> None: self._score_changed(copy_id, label, previous, current, remark=True) @@ -2243,6 +2270,8 @@ class CopienatorApp(tk.Tk): self.complete_giving_names_button.configure( state="disabled" if running else "normal" ) + if self.general_remarks_panel is not None: + self.general_remarks_panel.set_busy(running) if self.score_editor_panel is not None: self.score_editor_panel.set_busy(running) diff --git a/copienator_gui/general_remarks.py b/copienator_gui/general_remarks.py new file mode 100644 index 0000000..0b970d7 --- /dev/null +++ b/copienator_gui/general_remarks.py @@ -0,0 +1,137 @@ +"""Editor for ordered, copy-level LaTeX remarks.""" +import tkinter as tk +from tkinter import messagebox, ttk + +from copienator.general_remarks import remarks_for_copy, set_remarks +from .scores import score_path_for_id + + +class GeneralRemarksPanel(ttk.Frame): + def __init__(self, parent, evaluation, on_change): + super().__init__(parent) + self.evaluation, self.on_change = evaluation, on_change + self.busy = False + self.valid = False + self.remarks = [] + self.id_var = tk.StringVar() + self.status_var = tk.StringVar() + self.columnconfigure(1, weight=1) + ttk.Label(self, text="Id de copie").grid(row=0, column=0, sticky="w") + self.id_entry = ttk.Entry(self, textvariable=self.id_var) + self.id_entry.grid(row=0, column=1, sticky="ew") + self.listbox = tk.Listbox(self, height=4, exportselection=False) + self.listbox.grid(row=1, column=0, columnspan=2, sticky="ew", pady=6) + self.listbox.bind("<>", self.select) + self.editor = tk.Text(self, height=5, width=50, wrap="word", undo=True) + self.editor.grid(row=2, column=0, columnspan=2, sticky="ew") + actions = ttk.Frame(self) + actions.grid(row=3, column=0, columnspan=2, sticky="w", pady=6) + self.buttons = [] + for text, command in ( + ("Nouvelle", self.new), ("Enregistrer", self.save), + ("Supprimer", self.delete), ("Monter", lambda: self.move(-1)), + ("Descendre", lambda: self.move(1)), + ): + button = ttk.Button(actions, text=text, command=command) + button.pack(side="left", padx=(0, 5)) + self.buttons.append(button) + ttk.Label(self, text="Texte LaTeX ; chaque remarque forme un paragraphe.").grid( + row=4, column=0, columnspan=2, sticky="w") + ttk.Label(self, textvariable=self.status_var, wraplength=650).grid( + row=5, column=0, columnspan=2, sticky="w", pady=6) + self.id_var.trace_add("write", self.load) + self.load() + + def load(self, *_): + self.valid = False + self.remarks = [] + try: + path = score_path_for_id(self.evaluation, self.id_var.get()) + self.remarks = remarks_for_copy(self.evaluation, self.id_var.get()) + self.valid = True + self.status_var.set(path.parent.name) + except (OSError, ValueError) as exc: + self.status_var.set(str(exc)) + self.refresh() + self.set_busy(self.busy) + + def refresh(self, index=None): + self.listbox.configure(state="normal") + self.listbox.delete(0, "end") + for i, text in enumerate(self.remarks, 1): + self.listbox.insert("end", f"{i}. {' '.join(text.split())}") + self.editor.configure(state="normal") + self.editor.delete("1.0", "end") + if index is not None: + self.listbox.selection_set(index) + self.listbox.see(index) + self.editor.insert("1.0", self.remarks[index]) + self.editor.edit_reset() + + def select(self, *_): + if self.busy: + return + selected = self.listbox.curselection() + if selected: + self.editor.delete("1.0", "end") + self.editor.insert("1.0", self.remarks[selected[0]]) + self.editor.edit_reset() + + def new(self): + if self.valid and not self.busy: + self.refresh() + self.editor.focus_set() + + def commit(self, remarks, index=None): + try: + previous = set_remarks(self.evaluation, self.id_var.get(), remarks) + self.remarks = remarks + self.refresh(index) + if previous != remarks: + self.on_change(self.id_var.get(), previous, remarks) + self.status_var.set("Remarques enregistrées — lecture des annotations à revalider, « Régénérer » coché.") + except (OSError, ValueError) as exc: + messagebox.showerror("Remarques générales", str(exc)) + + def save(self): + if not self.valid or self.busy: + return + text = self.editor.get("1.0", "end-1c").strip() + if not text: + self.status_var.set("Saisissez une remarque ou utilisez « Supprimer ».") + return + remarks = self.remarks.copy() + selected = self.listbox.curselection() + index = selected[0] if selected else len(remarks) + if selected: + remarks[index] = text + else: + remarks.append(text) + self.commit(remarks, index) + + def delete(self): + selected = self.listbox.curselection() + if self.valid and not self.busy and selected: + remarks = self.remarks.copy() + remarks.pop(selected[0]) + self.commit(remarks) + + def move(self, offset): + selected = self.listbox.curselection() + if not self.valid or self.busy or not selected: + return + index = selected[0] + target = index + offset + if 0 <= target < len(self.remarks): + remarks = self.remarks.copy() + remarks[index], remarks[target] = remarks[target], remarks[index] + self.commit(remarks, target) + + def set_busy(self, busy): + self.busy = busy + self.id_entry.configure(state="disabled" if busy else "normal") + state = "normal" if self.valid and not busy else "disabled" + self.listbox.configure(state=state) + self.editor.configure(state=state) + for button in self.buttons: + button.configure(state=state) diff --git a/copienator_gui/workflow.py b/copienator_gui/workflow.py index e2a6bae..754a680 100644 --- a/copienator_gui/workflow.py +++ b/copienator_gui/workflow.py @@ -765,6 +765,17 @@ def build_workflow(show_personal_steps: bool) -> list[StepDefinition]: optional=True, requires=("A Rendre",), ), + StepDefinition( + "general_remarks", + "Finalisation", + "Remarques générales", + "Ajoutez, modifiez, supprimez ou réordonnez les remarques générales de chaque copie. " + "Enregistrez chaque remarque avant de changer de sélection. " + "Elles seront composées en LaTeX en tête des sorties lors de la régénération.", + (manual("default"),), + optional=True, + requires=("A Rendre",), + ), StepDefinition( "personal_create", "Étapes personnelles", diff --git a/tests/test_annotation_modifications.py b/tests/test_annotation_modifications.py index 0668505..8264d4c 100644 --- a/tests/test_annotation_modifications.py +++ b/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") diff --git a/tests/test_general_remarks.py b/tests/test_general_remarks.py new file mode 100644 index 0000000..85dec3f --- /dev/null +++ b/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) diff --git a/tests/test_gui_scores.py b/tests/test_gui_scores.py index 270d921..e369a7f 100644 --- a/tests/test_gui_scores.py +++ b/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",)) diff --git a/tests/test_incremental_annotation_reads.py b/tests/test_incremental_annotation_reads.py index a34f7c0..9680acf 100644 --- a/tests/test_incremental_annotation_reads.py +++ b/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) diff --git a/tests/test_left_margin.py b/tests/test_left_margin.py index b962675..ab1a65e 100644 --- a/tests/test_left_margin.py +++ b/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) diff --git a/tests/test_margin_benchmark.py b/tests/test_margin_benchmark.py new file mode 100644 index 0000000..aea1fe3 --- /dev/null +++ b/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()