Manual score modification step

copienator_gui/app.py

@@ -26,6 +26,7 @@ from copienator.rubrics import present_rubric_labels
 from .diagnostics import collect_diagnostics
 from .batch_monitor import BatchMonitor
 from .giving_names import GivingNamesPanel, find_name_issues
+from .scores import ScoreEditorPanel
 from .notifications import notify_desktop
 from .syncthing import SyncthingPanel
 from .manual_resolution import ManualResolutionPanel
@@ -279,6 +280,7 @@ class CopienatorApp(tk.Tk):
         self._rendering = False
         self.refaire_panel: RefaireSelection | None = None
         self.giving_names_panel: GivingNamesPanel | None = 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
         self.pending_refaire_commands: list[list[str]] = []
@@ -809,7 +811,7 @@ class CopienatorApp(tk.Tk):
         if not step:
             return
         self._rendering = True
-        if step.id == "personal_read":
+        if step.id in {"personal_read", "edit_scores"}:
             self.command_box.grid_remove()
         else:
             self.command_box.grid()
@@ -822,6 +824,7 @@ class CopienatorApp(tk.Tk):
             "plotting",
             "manual_resolution",
             "giving_names",
+            "edit_scores",
             "statement_persp",
         }
         self.console.configure(height=8 if expanded_form else 14)
@@ -835,6 +838,7 @@ class CopienatorApp(tk.Tk):
         self.arg_vars.clear()
         self.refaire_panel = None
         self.giving_names_panel = None
+        self.score_editor_panel = None
         self.complete_giving_names_button = None
         self.label_groups_button = None
         entry = self.state_store.step(step.id) if self.state_store.evaluation else {}
@@ -1050,6 +1054,12 @@ class CopienatorApp(tk.Tk):
                 )
                 row += 1
             return row
+        if step.id == "edit_scores" and self.evaluation:
+            self.score_editor_panel = ScoreEditorPanel(
+                self.form, self.evaluation, self._score_changed
+            )
+            self.score_editor_panel.grid(row=row, column=0, columnspan=2, sticky="ew", pady=8)
+            row += 1
         if step.id == "giving_names" and self.evaluation:
             self.giving_names_panel = GivingNamesPanel(
                 self.form, self.evaluation
@@ -1658,6 +1668,8 @@ class CopienatorApp(tk.Tk):
         if not self.current_step or not self.state_store.evaluation or self.active_step_id:
             return
         self._save_current_form()
+        # Score edits already invalidate downstream steps when Set saves a change.
+        if self.current_step.id != "edit_scores":
             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] = {
@@ -1687,6 +1699,20 @@ class CopienatorApp(tk.Tk):
         self._populate_tree()
         self.info_var.set("Renommage validé — aucun fichier modifié.")
 
+    def _score_changed(self, copy_id: str, label: str, previous: str, current: str) -> None:
+        step = self.step_by_id["edit_scores"]
+        self._invalidate_after_step(step, "default")
+        self.state_store.update_step(step.id, status="detected")
+        self.state_store.add_history({
+            "step": step.id, "copy_id": copy_id, "label": label,
+            "previous_score": previous, "score": current, "manual": True,
+        })
+        # Refresh statuses without rebuilding the form between score edits.
+        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 _complete_giving_names(self) -> None:
         if (
             not self.current_step
@@ -2091,6 +2117,8 @@ class CopienatorApp(tk.Tk):
             self.complete_giving_names_button.configure(
                 state="disabled" if running else "normal"
             )
+        if self.score_editor_panel is not None:
+            self.score_editor_panel.set_busy(running)
 
     def _move_selection(self, delta: int) -> None:
         if not self.current_step:

copienator_gui/scores.py

@@ -0,0 +1,135 @@
+from __future__ import annotations
+
+import re
+import tkinter as tk
+from decimal import Decimal, InvalidOperation
+from pathlib import Path
+from tkinter import messagebox, ttk
+
+from copienator import EvaluationWorkspace, atomic_update_json, read_json
+from copienator.utils import natural_key
+from .giving_names import RETURN_FOLDER_PATTERN
+
+
+def score_path_for_id(evaluation: Path, copy_id: str) -> Path:
+    identifier = copy_id.strip()
+    if not re.fullmatch(r"[0-9]+", identifier):
+        raise ValueError("Saisissez un numéro d’id.")
+    return_dir = EvaluationWorkspace(evaluation).return_dir
+    matches = []
+    if return_dir.is_dir():
+        for folder in return_dir.iterdir():
+            match = RETURN_FOLDER_PATTERN.fullmatch(folder.name)
+            if folder.is_dir() and not folder.is_symlink() and match and int(match["copy_id"]) == int(identifier):
+                matches.append(folder / "score.json")
+    if not matches:
+        raise FileNotFoundError(f"Aucun dossier dans A Rendre pour l’id {identifier}.")
+    if len(matches) > 1:
+        raise ValueError(f"Plusieurs dossiers dans A Rendre portent l’id {identifier}.")
+    if not matches[0].is_file():
+        raise FileNotFoundError(f"score.json introuvable : {matches[0]}")
+    return matches[0]
+
+
+def read_scores(path: Path) -> dict:
+    scores = read_json(path)
+    if not isinstance(scores, dict):
+        raise ValueError(f"score.json doit contenir un objet JSON : {path}")
+    return scores
+
+
+def set_score(path: Path, label: str, value: str) -> str:
+    try:
+        number = Decimal(value.strip().replace(",", "."))
+    except InvalidOperation:
+        raise ValueError("Le score doit être un nombre.") from None
+    if not number.is_finite() or number < 0:
+        raise ValueError("Le score doit être un nombre positif ou nul.")
+    previous = ""
+
+    def update(scores):
+        nonlocal previous
+        if not isinstance(scores, dict) or label not in scores:
+            raise ValueError(f"Label absent de score.json : {label}")
+        previous = str(scores[label])
+        scores[label] = str(number)
+
+    # A Rendre/score.json is often a symlink to the annotation score file.
+    # Resolve it before atomic replacement so the link continues to work.
+    atomic_update_json(path.resolve(strict=True), update, default_factory=dict)
+    return previous
+
+
+class ScoreEditorPanel(ttk.Frame):
+    def __init__(self, parent, evaluation: Path, on_change):
+        super().__init__(parent)
+        self.evaluation = evaluation
+        self.on_change = on_change
+        self.current_path = None
+        self.busy = False
+        self.id_var = tk.StringVar()
+        self.label_var = tk.StringVar()
+        self.score_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", padx=(0, 8), pady=4)
+        self.id_entry = ttk.Entry(self, textvariable=self.id_var)
+        self.id_entry.grid(row=0, column=1, sticky="ew", pady=4)
+        ttk.Label(self, text="Label").grid(row=1, column=0, sticky="w", padx=(0, 8), pady=4)
+        self.label_entry = ttk.Combobox(self, textvariable=self.label_var)
+        self.label_entry.grid(row=1, column=1, sticky="ew", pady=4)
+        ttk.Label(self, text="Score").grid(row=2, column=0, sticky="w", padx=(0, 8), pady=4)
+        self.score_entry = ttk.Entry(self, textvariable=self.score_var)
+        self.score_entry.grid(row=2, column=1, sticky="ew", pady=4)
+        self.set_button = ttk.Button(self, text="Set", command=self.save_score)
+        self.set_button.grid(row=2, column=2, padx=(8, 0), pady=4)
+        ttk.Label(self, textvariable=self.status_var, wraplength=650).grid(
+            row=3, column=0, columnspan=3, sticky="w", pady=8
+        )
+        self.id_var.trace_add("write", self.load_score)
+        self.label_var.trace_add("write", self.load_score)
+        self.score_entry.bind("<Return>", lambda _event: self.save_score())
+        self.load_score()
+
+    def load_score(self, *_args):
+        self.current_path = None
+        self.score_var.set("")
+        self.label_entry.configure(values=())
+        try:
+            path = score_path_for_id(self.evaluation, self.id_var.get())
+            scores = read_scores(path)
+            self.label_entry.configure(values=sorted(scores, key=natural_key))
+            label = self.label_var.get().strip()
+            if not label:
+                self.status_var.set("Choisissez ou saisissez un label.")
+            elif label not in scores:
+                self.status_var.set(f"Label absent de score.json : {label}")
+            else:
+                self.current_path = path
+                self.score_var.set(str(scores[label]))
+                self.status_var.set(path.parent.name)
+        except (OSError, ValueError) as exc:
+            self.status_var.set(str(exc))
+        self.set_busy(self.busy)
+
+    def set_busy(self, busy: bool):
+        self.busy = busy
+        self.id_entry.configure(state="disabled" if busy else "normal")
+        self.label_entry.configure(state="disabled" if busy else "normal")
+        state = "normal" if self.current_path is not None and not busy else "disabled"
+        self.score_entry.configure(state=state)
+        self.set_button.configure(state=state)
+
+    def save_score(self):
+        if self.current_path is None or self.busy:
+            return
+        label = self.label_var.get().strip()
+        try:
+            previous = set_score(self.current_path, label, self.score_var.get())
+            current = str(read_scores(self.current_path)[label])
+            self.score_var.set(current)
+            if previous != current:
+                self.on_change(self.id_var.get().strip(), label, previous, current)
+            self.status_var.set(f"Score enregistré : {self.current_path.parent.name} — {label} : {current}")
+        except (OSError, ValueError) as exc:
+            messagebox.showerror("Modification du score", str(exc))

copienator_gui/workflow.py

@@ -679,6 +679,17 @@ def build_workflow(show_personal_steps: bool) -> list[StepDefinition]:
             ),
             artifacts=("A Rendre",),
         ),
+        StepDefinition(
+            "edit_scores",
+            "Finalisation",
+            "Modifier des scores",
+            "Saisissez l’id de copie, puis choisissez ou saisissez un label. "
+            "Le score actuel est chargé automatiquement. Modifiez-le puis cliquez sur Set "
+            "pour l’enregistrer dans score.json. Cliquez sur « Marquer terminée » quand vous avez fini.",
+            (manual("default"),),
+            optional=True,
+            requires=("A Rendre",),
+        ),
         StepDefinition(
             "personal_create",
             "Étapes personnelles",

tests/test_annotation_import_cleanup.py

@@ -0,0 +1,94 @@
+import tempfile
+import unittest
+from contextlib import ExitStack
+from pathlib import Path
+from unittest.mock import patch
+
+from copienator import CliError, EvaluationWorkspace, ExitCode
+from copienator.annotation_data import AnnotationLoadResult
+from copienator.commands import annotating as simple
+from copienator.commands import annotating_by_label as grouped
+from copienator.commands import annotating_with_checks as checks
+from copienator.commands import import_annotations
+
+
+class AnnotationImportCleanupTests(unittest.TestCase):
+    def setUp(self):
+        temporary = tempfile.TemporaryDirectory()
+        self.addCleanup(temporary.cleanup)
+        self.root = Path(temporary.name)
+        self.workspace = EvaluationWorkspace(self.root / "Exam")
+        for directory in ("Copies", "Par label"):
+            (self.workspace.root / directory).mkdir(parents=True)
+        (self.workspace.root / "labels").write_text("A\n")
+        (self.workspace.root / "correction.json").write_text("{}")
+        self.import_dir = self.root / "Annotées"
+        self.import_dir.mkdir()
+        self.patch = patch.object(import_annotations, "IMPORT_DIR", self.import_dir)
+        self.patch.start()
+        self.addCleanup(self.patch.stop)
+
+    def test_cleanup_empties_folder_without_following_child_symlinks(self):
+        (self.import_dir / "old.pdf").write_bytes(b"return")
+        nested = self.import_dir / "nested"
+        nested.mkdir()
+        (nested / "old.jpg").write_bytes(b"return")
+        outside = self.root / "keep"
+        outside.mkdir()
+        (outside / "keep.pdf").write_bytes(b"keep")
+        (self.import_dir / "link").symlink_to(outside, target_is_directory=True)
+        import_annotations.clear_import_directory(self.workspace)
+        self.assertTrue(self.import_dir.is_dir())
+        self.assertEqual(list(self.import_dir.iterdir()), [])
+        self.assertTrue((outside / "keep.pdf").is_file())
+
+    def test_cleanup_handles_unconfigured_or_missing_folder(self):
+        for configured in (None, "", self.root / "absent"):
+            with self.subTest(configured=configured), \
+                    patch.object(import_annotations, "IMPORT_DIR", configured):
+                import_annotations.clear_import_directory(self.workspace)
+        self.assertFalse((self.root / "absent").exists())
+
+    def test_cleanup_rejects_an_ancestor_of_evaluation(self):
+        with patch.object(import_annotations, "IMPORT_DIR", self.root), \
+                self.assertRaises(CliError):
+            import_annotations.clear_import_directory(self.workspace)
+        self.assertTrue(self.workspace.root.is_dir())
+
+    def test_all_modes_clean_only_when_checked_and_successful(self):
+        for module in (simple, checks, grouped):
+            for clear_import, warnings in ((False, []), (True, []), (True, ["missing answer"])):
+                with self.subTest(mode=module.__name__, checked=clear_import, warnings=warnings):
+                    old = self.import_dir / "old.pdf"
+                    old.write_bytes(b"return")
+                    with ExitStack() as patches:
+                        patches.enter_context(patch.object(
+                            module, "load_annotation_data",
+                            return_value=AnnotationLoadResult({"01": {"A": {}}}, warnings),
+                        ))
+                        if module is simple:
+                            patches.enter_context(patch.object(
+                                simple, "process_correction", return_value=ExitCode.SUCCESS
+                            ))
+                        elif module is checks:
+                            patches.enter_context(patch.object(
+                                checks, "_render_student", return_value="success"
+                            ))
+                        else:
+                            patches.enter_context(patch.object(
+                                grouped, "_generate_groups", return_value=(1, 0)
+                            ))
+                            patches.enter_context(patch.object(
+                                grouped, "_load_label_groups", return_value=[["A"]]
+                            ))
+                        targets = (self.workspace.root,) if module is checks else ()
+                        status = module.run(self.workspace, *targets, clear_import=clear_import)
+                    self.assertEqual(status, ExitCode.PARTIAL if warnings else ExitCode.SUCCESS)
+                    self.assertEqual(old.exists(), not (clear_import and not warnings))
+
+    def test_cli_forwards_cleanup_flag_for_each_mode(self):
+        for module in (simple, checks, grouped):
+            with self.subTest(mode=module.__name__), \
+                    patch.object(module, "run", return_value=ExitCode.SUCCESS) as run:
+                self.assertEqual(module.main([str(self.workspace.root), "--clear-import"]), 0)
+                self.assertTrue(run.call_args.kwargs["clear_import"])

tests/test_gui_scores.py

@@ -0,0 +1,113 @@
+import os
+import tempfile
+import unittest
+from pathlib import Path
+from unittest.mock import patch
+
+from copienator import read_json
+from copienator_gui.app import CopienatorApp
+from copienator_gui.scores import score_path_for_id, set_score
+from copienator_gui.workflow import build_workflow
+
+
+class ScoreEditorTests(unittest.TestCase):
+    def setUp(self):
+        temporary = tempfile.TemporaryDirectory()
+        self.addCleanup(temporary.cleanup)
+        self.evaluation = Path(temporary.name)
+        folder = self.evaluation / "A Rendre" / "Student (01)"
+        folder.mkdir(parents=True)
+        self.path = folder / "score.json"
+        self.path.write_text('{"Ex 1": "2", "Ex 2": "4"}')
+
+    def test_step_is_optional_and_follows_return_preparation_in_both_profiles(self):
+        for personal in (False, True):
+            steps = build_workflow(personal)
+            index = next(index for index, step in enumerate(steps) if step.id == "giving_names")
+            self.assertEqual(steps[index + 1].id, "edit_scores")
+            self.assertTrue(steps[index + 1].optional)
+            self.assertTrue(steps[index + 1].is_manual)
+
+    def test_update_preserves_other_labels_and_follows_symlinks(self):
+        source = self.evaluation / "source-score.json"
+        self.path.rename(source)
+        self.path.symlink_to(source)
+        path = score_path_for_id(self.evaluation, "1")
+        self.assertEqual(path, score_path_for_id(self.evaluation, "01"))
+        self.assertEqual(set_score(path, "Ex 1", "3,5"), "2")
+        self.assertTrue(path.is_symlink())
+        self.assertEqual(read_json(source), {"Ex 1": "3.5", "Ex 2": "4"})
+        self.assertEqual(read_json(path), read_json(source))
+
+    def test_invalid_score_or_missing_label_does_not_modify_file(self):
+        before = self.path.read_bytes()
+        for value in ("", "abc", "NaN", "Infinity", "-1"):
+            with self.subTest(value=value), self.assertRaises(ValueError):
+                set_score(self.path, "Ex 1", value)
+            self.assertEqual(self.path.read_bytes(), before)
+        with self.assertRaises(ValueError):
+            set_score(self.path, "Missing", "3")
+        self.assertEqual(self.path.read_bytes(), before)
+
+    def test_missing_and_ambiguous_ids_are_rejected(self):
+        with self.assertRaises(FileNotFoundError):
+            score_path_for_id(self.evaluation, "2")
+        with self.assertRaises(ValueError):
+            score_path_for_id(self.evaluation, "../1")
+        (self.path.parent.parent / "Other (1)").mkdir()
+        with self.assertRaises(ValueError):
+            score_path_for_id(self.evaluation, "1")
+
+
+@unittest.skipUnless(os.environ.get("DISPLAY"), "Tk requires a display")
+class ScoreEditorGuiTests(ScoreEditorTests):
+    def setUp(self):
+        super().setUp()
+        self.app = CopienatorApp(Path.cwd(), True, self.evaluation)
+        self.addCleanup(self.close_app)
+        self.app.update()
+        self.app.tree.selection_set("edit_scores")
+        self.app.update()
+        self.panel = self.app.score_editor_panel
+
+    def close_app(self):
+        for callback in self.app.tk.splitlist(self.app.tk.call("after", "info")):
+            self.app.after_cancel(callback)
+        self.app.destroy()
+
+    def test_selection_loads_score_and_set_keeps_editor_open(self):
+        self.assertEqual(str(self.panel.set_button.cget("state")), "disabled")
+        self.panel.id_var.set("1")
+        self.assertEqual(tuple(self.panel.label_entry.cget("values")), ("Ex 1", "Ex 2"))
+        self.panel.label_var.set("Ex 1")
+        self.assertEqual(self.panel.score_var.get(), "2")
+        self.app.state_store.update_step("final_score", status="success")
+        self.panel.score_var.set("3,5")
+        self.panel.set_button.invoke()
+        self.app.update()
+        self.assertIs(self.app.score_editor_panel, self.panel)
+        self.assertEqual(self.panel.id_var.get(), "1")
+        self.assertEqual(read_json(self.path), {"Ex 1": "3.5", "Ex 2": "4"})
+        self.assertEqual(self.app.state_store.step("final_score")["status"], "stale")
+        self.panel.label_var.set("Ex 2")
+        self.assertEqual(self.panel.score_var.get(), "4")
+        self.panel.id_var.set("2")
+        self.assertEqual(self.panel.score_var.get(), "")
+        self.assertEqual(str(self.panel.set_button.cget("state")), "disabled")
+
+    def test_invalid_score_shows_error_and_running_disables_edits(self):
+        self.panel.id_var.set("01")
+        self.panel.label_var.set("Ex 1")
+        self.panel.score_var.set("bad")
+        with patch("copienator_gui.scores.messagebox.showerror") as error:
+            self.panel.set_button.invoke()
+        error.assert_called_once()
+        self.assertEqual(read_json(self.path)["Ex 1"], "2")
+        self.panel.set_busy(True)
+        self.assertEqual(str(self.panel.set_button.cget("state")), "disabled")
+
+    def test_skipping_without_changes_preserves_downstream_status(self):
+        self.app.state_store.update_step("final_score", status="success")
+        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")