diff --git a/.gitignore b/.gitignore index f23ec68..b1e0bad 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,6 @@ OLD/ +/Interro*/ +/DS*/ __pycache__/ *.py[cod] .venv/ diff --git a/Readme.org b/Readme.org index ac3c67c..5eee572 100644 --- a/Readme.org +++ b/Readme.org @@ -197,13 +197,17 @@ racine du projet). Dans la console, les boutons de copie et le clic droit permettent de copier la sélection ou toute la sortie ; =Ctrl+C= et =Ctrl+A= sont également disponibles (=Cmd= sous macOS). -Pendant =Découper la marge des labels=, =s= signale la copie en erreur -et passe à la suivante. =Entrée= valide et enregistre la découpe affichée. +Pendant =Découper une partie à gauche pour détection des labels=, =n= décale +la zone de 50 px vers la droite, =N= de 100 px, =t= de 50 px vers la +gauche et =l= l’élargit de 50 px. =1= utilise les pages entières. =s= +signale la copie en erreur et passe à la suivante ; =Entrée= valide et +enregistre la découpe affichée. Une copie ignorée conserve ses anciennes découpes. Les signalements sont conservés dans =.copienator/copy_errors.json=, même après fermeture. Dans =Séparer et réordonner les pages=, =Traiter les copies signalées= reprend ces copies à partir des originaux conservés. Le même bouton dans -=Découper la marge des labels= reprend leurs marges ; chaque signalement +=Découper une partie à gauche pour détection des labels= reprend leur +découpage ; chaque signalement est effacé seulement après validation et enregistrement avec =Entrée=. Fermer la fenêtre, appuyer à nouveau sur =s= ou rencontrer une erreur conserve le signalement. Les commandes =page-split= et =crop-labels= diff --git a/copienator/commands/correction.py b/copienator/commands/correction.py index 42e1c7b..f8fc5d6 100644 --- a/copienator/commands/correction.py +++ b/copienator/commands/correction.py @@ -5,6 +5,7 @@ import base64 import concurrent.futures import json import os +import signal import shlex import shutil import sys @@ -45,15 +46,22 @@ COPIES_DIR = Path() GROUPS_DIR = Path() output_path = Path() progress_path = Path() +pending_responses_path = Path() tasks: list[tuple] = [] tasks_to_process: list[tuple] = [] results: dict = {} completed_tasks: list = [] errors_summary: list = [] +pending_responses: dict[str, str] = {} overwrite = False limit = None client = None start_time = 0.0 +stop_requested = threading.Event() + + +class CorrectionStopRequested(Exception): + """Raised in a worker before it starts another Gemini request.""" # --- Thread-safe Logging --- log_lock = threading.Lock() @@ -82,6 +90,25 @@ def flush_thread_log(tid=None): f.write("\n".join(thread_logs[tid]) + "\n\n") thread_logs[tid].clear() + +def report_group_progress(completed: int, total: int) -> None: + """Emit a stable, human-readable progress line for the GUI and CLI.""" + print( + f"[Progression correction] Groupes traités : {completed}/{total}", + flush=True, + ) + + +def request_graceful_stop(_signum=None, _frame=None) -> None: + """Stop scheduling Gemini calls while allowing in-flight calls to finish.""" + if not stop_requested.is_set(): + stop_requested.set() + print( + "\n[Interruption] Arrêt des nouveaux appels Gemini demandé. " + "Attente des appels en cours et sauvegarde de leurs résultats…", + flush=True, + ) + # --- Lock for thread-safe file writing --- io_lock = threading.Lock() pro_lock = threading.Lock() @@ -140,6 +167,7 @@ def configure_runtime( api_client=None, ) -> None: global INPUT_DIR, COPIES_DIR, GROUPS_DIR, output_path, progress_path + global pending_responses_path, pending_responses global tasks, tasks_to_process, results, completed_tasks, errors_summary global overwrite, limit, client, start_time global pro_count, flash_count, pro_quota_exhausted @@ -149,11 +177,13 @@ def configure_runtime( GROUPS_DIR = workspace.groups_dir output_path = workspace.correction_file progress_path = workspace.correction_progress_file + pending_responses_path = workspace.correction_pending_responses_file tasks = list(discovered_tasks) overwrite = bool(args.overwrite) limit = args.limit start_time = time.time() errors_summary = [] + pending_responses = {} completed_tasks = [] results = {label: [] for _file, label in tasks} thread_logs.clear() @@ -161,6 +191,7 @@ def configure_runtime( pro_count = 0 flash_count = 0 pro_quota_exhausted = False + stop_requested.clear() if not overwrite: if progress_path.is_file(): @@ -174,6 +205,20 @@ def configure_runtime( raise TypeError("correction.json must contain a JSON object") results = loaded_results + # A response saved during a graceful interruption is not a completed + # correction. Resume its auxiliary checks even with --overwrite instead of + # paying for the same primary request again. + if pending_responses_path.is_file(): + loaded_pending = read_json(pending_responses_path) + if not isinstance(loaded_pending, dict) or not all( + isinstance(key, str) and isinstance(value, str) + for key, value in loaded_pending.items() + ): + raise TypeError( + "correction_pending_responses.json must contain a JSON object" + ) + pending_responses = loaded_pending + completed_set = {(str(file_path), label) for file_path, label in completed_tasks} tasks_to_process = [ task for task in tasks if (str(task[0]), task[1]) not in completed_set @@ -184,7 +229,11 @@ def configure_runtime( def reset_workspace(workspace: EvaluationWorkspace) -> None: """Apply the explicitly requested correction reset.""" print("--- Running Reset ---") - for path in (workspace.correction_file, workspace.correction_progress_file): + for path in ( + workspace.correction_file, + workspace.correction_progress_file, + workspace.correction_pending_responses_file, + ): if path.exists(): path.unlink() print(f"Deleted: {path}") @@ -213,6 +262,8 @@ def call_gemini_with_retries(model_id, contents, config, delays = [60, 300] for attempt in range(3): + if stop_requested.is_set(): + raise CorrectionStopRequested # Switch to fallback immediately if quota was exhausted by another thread if model_id == MODEL_ID_pro and pro_quota_exhausted and fallback_model_id: model_id = fallback_model_id @@ -228,6 +279,8 @@ def call_gemini_with_retries(model_id, contents, config, full_response_text += chunk.text return full_response_text except Exception as e: + if stop_requested.is_set(): + raise CorrectionStopRequested from e error_msg = str(e).lower() is_quota_error = "429" in error_msg or "quota" in error_msg or "exhausted" in error_msg is_minute_limit = "minute" in error_msg or "rpm" in error_msg or "tpm" in error_msg @@ -239,7 +292,8 @@ def call_gemini_with_retries(model_id, contents, config, wait_time = float(retry_match.group(1)) + 1.0 if retry_match else delays[attempt] tprint(f"\tGemini Pro minute limit hit. Waiting {wait_time:.1f}s...") - time.sleep(wait_time) + if stop_requested.wait(wait_time): + raise CorrectionStopRequested continue # Retry same model # Immediately fallback to Flash without waiting if it's a Pro quota error @@ -251,7 +305,8 @@ def call_gemini_with_retries(model_id, contents, config, if attempt < 2: tprint(f"\tGemini API failure: {e}. Retrying in {delays[attempt]} seconds...") - time.sleep(delays[attempt]) + if stop_requested.wait(delays[attempt]): + raise CorrectionStopRequested else: tprint(f"\tGemini API failure: {e}. Maximum retries reached.") raise @@ -359,6 +414,8 @@ def handle_label_errors(pid, label, res, pdf_path): tprint(f"\tHandling additional-answer for {pid} {label}") try: add_labels = json.loads(call_gemini_with_retries(MODEL_ID_flash, contents, config)) + except CorrectionStopRequested: + raise except Exception: # noqa: BLE001 - invalid auxiliary model response add_labels = [] @@ -417,6 +474,9 @@ def process_single_task(task_tuple, precomputed_response=None): total_height = group_data[-1][2] use_flash = n >= 4 or total_height <= 500 + if precomputed_response is None and stop_requested.is_set(): + raise CorrectionStopRequested + # Only apply limits and counts if we are making a live call if precomputed_response is None: if not use_flash: @@ -437,9 +497,11 @@ def process_single_task(task_tuple, precomputed_response=None): model_to_use = MODEL_ID_flash if use_flash else MODEL_ID_pro if precomputed_response: - tprint(f"Using batched response for: {label} {group_name}") + tprint(f"Using saved response for: {label} {group_name}") full_response_text = precomputed_response else: + if stop_requested.is_set(): + raise CorrectionStopRequested tprint(f"Asking Gemini {'Flash' if use_flash else 'Pro '}: {label} {group_name}") full_response_text = call_gemini_with_retries(model_to_use, contents, config) @@ -478,8 +540,28 @@ def process_single_task(task_tuple, precomputed_response=None): if res["error"] != "": tprint("\tError :", res["error"], "for Copie", pid, group_name) - if can_spawn_tasks and res.get("error") in ["wrong-label", "additional-answer"]: - new_tasks.extend(handle_label_errors(pid, label, res, pdf_path)) + if can_spawn_tasks and res.get("error") in [ + "wrong-label", + "additional-answer", + ]: + if stop_requested.is_set(): + with io_lock: + pending_responses[file_path] = json.dumps(json_data) + atomic_write_json( + pending_responses_path, pending_responses + ) + raise CorrectionStopRequested + try: + new_tasks.extend( + handle_label_errors(pid, label, res, pdf_path) + ) + except CorrectionStopRequested: + with io_lock: + pending_responses[file_path] = json.dumps(json_data) + atomic_write_json( + pending_responses_path, pending_responses + ) + raise # Si "wrong-label" a déplacé le fichier courant vers _old if res.get("error", "").startswith("wrg-lbl-moved-to:"): current_suffix = "_old" @@ -520,6 +602,8 @@ def process_single_task(task_tuple, precomputed_response=None): if needs_correction: tprint(f"\tBox anomalies detected for Copie {pid} {group_name}. \n\tRequesting isolated correction from Gemini Flash...") try: + if stop_requested.is_set(): + raise CorrectionStopRequested # Pensez à passer pdf_path à la fonction modifiée ! res["feedback"] = correct_boxes_with_gemini( pid, label, pdf_path, res["feedback"], @@ -542,7 +626,12 @@ def process_single_task(task_tuple, precomputed_response=None): # To track progress completed_tasks.append((file_path, label)) atomic_write_json(progress_path, completed_tasks) + if file_path in pending_responses: + del pending_responses[file_path] + atomic_write_json(pending_responses_path, pending_responses) + except CorrectionStopRequested: + raise except json.JSONDecodeError: tprint(f"Error decoding JSON for {file_path}", file=sys.stderr) with io_lock: @@ -836,36 +925,82 @@ def run_configured(args: argparse.Namespace) -> ExitCode: else: print(f"Warning: Batch results file {batch_results_path} not found.", file=sys.stderr) + report_live_progress = not any( + (args.batch, args.batch_from, args.deal_with_batched, args.refaire) + ) + progress_total = len(tasks) + progress_completed = max(0, progress_total - len(tasks_to_process)) + if report_live_progress: + report_group_progress(progress_completed, progress_total) + made_progress = True while tasks_to_process or made_progress: if tasks_to_process: print(f"Starting processing on {len(tasks_to_process)} tasks with {NB_THREADS} threads...") with concurrent.futures.ThreadPoolExecutor(max_workers=NB_THREADS) as executor: + waiting_tasks = list(tasks_to_process) futures = {} - for task in tasks_to_process: - file_path = task[0] - precomp = batched_responses.get(file_path) - futures[executor.submit(process_single_task, task, precomp)] = task - for future in concurrent.futures.as_completed(futures): - try: - new_generated_tasks = future.result() - if new_generated_tasks: - for new_task in new_generated_tasks: - futures[executor.submit(process_single_task, new_task)] = new_task - except Exception as e: # noqa: BLE001 - future boundary - print(f"Exception during task execution: {e}", file=sys.stderr) - failed_task = futures[future] - with io_lock: - errors_summary.append((str(e), failed_task[0])) + def submit_available_tasks() -> None: + while ( + waiting_tasks + and len(futures) < NB_THREADS + and not stop_requested.is_set() + ): + task = waiting_tasks.pop(0) + file_path = task[0] + precomp = pending_responses.get( + file_path, batched_responses.get(file_path) + ) + futures[ + executor.submit(process_single_task, task, precomp) + ] = task + + submit_available_tasks() + while futures: + completed_futures, _pending = concurrent.futures.wait( + tuple(futures), + return_when=concurrent.futures.FIRST_COMPLETED, + ) + for future in completed_futures: + failed_task = futures.pop(future) + task_completed = False + try: + new_generated_tasks = future.result() + task_completed = True + if new_generated_tasks and not stop_requested.is_set(): + if report_live_progress: + progress_total += len(new_generated_tasks) + waiting_tasks.extend(new_generated_tasks) + except CorrectionStopRequested: + pass + except Exception as e: # noqa: BLE001 - future boundary + print( + f"Exception during task execution: {e}", + file=sys.stderr, + ) + with io_lock: + errors_summary.append((str(e), failed_task[0])) + if report_live_progress and task_completed: + progress_completed += 1 + report_group_progress( + progress_completed, progress_total + ) + submit_available_tasks() tasks_to_process = [] # Vider la liste une fois traitée # Après avoir traité toutes les tâches actuelles (live ou batched), # on tente de débloquer les mouvements qui étaient en attente + if stop_requested.is_set(): + break + delayed_tasks = resolve_delayed_moves() if delayed_tasks: print(f"Resolved {len(delayed_tasks)} delayed moves! Running executor for new tasks...") + if report_live_progress: + progress_total += len(delayed_tasks) + report_group_progress(progress_completed, progress_total) tasks_to_process.extend(delayed_tasks) made_progress = True else: @@ -907,6 +1042,12 @@ def run_configured(args: argparse.Namespace) -> ExitCode: print(err, file=sys.stderr) escaped_path = shlex.quote(str(file)) print(f"Run : python -m copienator correct {escaped_path}") + if stop_requested.is_set(): + print( + "[Interruption] Appels en cours terminés ; résultats disponibles sauvegardés.", + flush=True, + ) + return ExitCode.INTERRUPTED return ExitCode.PARTIAL if errors_summary else ExitCode.SUCCESS @@ -931,9 +1072,17 @@ def run( configure_runtime(workspace, discovered, args, api_client=api_client) if not discovered and not args.refaire: return ExitCode.PARTIAL + previous_handlers = {} + for signal_name in ("SIGINT", "SIGBREAK"): + interrupt_signal = getattr(signal, signal_name, None) + if interrupt_signal is not None: + previous_handlers[interrupt_signal] = signal.getsignal(interrupt_signal) + signal.signal(interrupt_signal, request_graceful_stop) try: status = run_configured(args) finally: + for interrupt_signal, previous_handler in previous_handlers.items(): + signal.signal(interrupt_signal, previous_handler) for thread_id in list(thread_logs): flush_thread_log(thread_id) if warnings and status == ExitCode.SUCCESS: @@ -983,6 +1132,12 @@ def main(argv: Sequence[str] | None = None) -> int: parser = build_parser() def handle(args: argparse.Namespace) -> ExitCode: + if args.refaire and args.overwrite: + raise CliError( + "--overwrite cannot be used with --refaire; --refaire already " + "replaces the corrections selected in refaire.json", + ExitCode.INVALID_ARGUMENTS, + ) workspace, target = workspace_from_target(args) targets = [target] for additional in args.additional_targets: diff --git a/copienator/commands/cutleft.py b/copienator/commands/cutleft.py index 5d1542a..f9b122c 100644 --- a/copienator/commands/cutleft.py +++ b/copienator/commands/cutleft.py @@ -28,6 +28,8 @@ from copienator.copy_errors import clear_copy_error, mark_copy_error, marked_cop DELIMITER_WIDTH = 5 DELIMITER_COLOR = (0, 0, 0) OUTPUT_SIZE = (1800, 1000) +CROP_SHIFT_STEP = 50 +CROP_WIDTH_STEP = 50 pdf_cache_lock = threading.Lock() @@ -80,6 +82,7 @@ def process_single_pdf( pdf_path: Path, shift_offset: int = 0, max_per_file: int = 5, + width_offset: int = 0, ) -> tuple[Image.Image, list[Image.Image], dict[str, object]] | None: """Convert one PDF into a preview, full-resolution splits and metadata.""" try: @@ -90,7 +93,10 @@ def process_single_pdf( left, right = 0, width else: left = max(0, 100 + shift_offset) - right = min(width, width // 3 + 100 + shift_offset) + right = min( + width, + width // 3 + 100 + shift_offset + max(0, width_offset), + ) if right > left: cropped_images.append(image.crop((left, 0, right, height))) if not cropped_images: @@ -166,6 +172,7 @@ class ImageReviewer: self.current_result = None self.index = 0 self.current_shift = 0 + self.current_width_offset = 0 self.default_max_per_file = default_max_per_file self.current_max_per_file = default_max_per_file self.current_preview: Image.Image | None = None @@ -184,9 +191,10 @@ class ImageReviewer: self.root.bind("", self.on_next) self.root.bind("s", self.on_skip) self.root.protocol("WM_DELETE_WINDOW", self.on_close) - self.root.bind("n", lambda _event: self.on_shift(50)) - self.root.bind("N", lambda _event: self.on_shift(100)) - self.root.bind("t", lambda _event: self.on_shift(-50)) + self.root.bind("n", lambda _event: self.on_shift(CROP_SHIFT_STEP)) + self.root.bind("N", lambda _event: self.on_shift(2 * CROP_SHIFT_STEP)) + self.root.bind("t", lambda _event: self.on_shift(-CROP_SHIFT_STEP)) + self.root.bind("l", lambda _event: self.on_enlarge(CROP_WIDTH_STEP)) self.root.bind("1", lambda _event: self.on_set_max_pages(1)) Thread(target=self.prefetch_worker, daemon=True).start() @@ -222,6 +230,7 @@ class ImageReviewer: return self.is_processing = False self.current_shift = 0 + self.current_width_offset = 0 self.current_result = None self.trigger_processing(self.files[self.index], self.current_shift) @@ -234,7 +243,12 @@ class ImageReviewer: def worker() -> None: self.manual_queue.put( - process_single_pdf(pdf_path, shift, self.current_max_per_file) + process_single_pdf( + pdf_path, + shift, + self.current_max_per_file, + self.current_width_offset, + ) ) Thread(target=worker, daemon=True).start() @@ -271,10 +285,12 @@ class ImageReviewer: self.label_info.configure( text=( f"[{self.index + 1}/{len(self.files)}] {filename} | " - f"Shift: {self.current_shift}px\nFiles: {schema['number_of_files']} | " + f"Shift: {self.current_shift}px | " + f"Extra width: {self.current_width_offset}px\n" + f"Files: {schema['number_of_files']} | " f"Cols: {schema['columns_per_file']}\n" "Enter: Save and next | s: flag error and skip | n: +50 | N: +100 | t: -50 | " - "1: use single column" + "l: widen by 50 | 1: use full pages" ), fg="black", ) @@ -286,6 +302,13 @@ class ImageReviewer: print(f"Applying shift: {self.current_shift}") self.trigger_processing(self.files[self.index], self.current_shift) + def on_enlarge(self, amount: int) -> None: + if self.is_processing: + return + self.current_width_offset += amount + print(f"Applying extra width: {self.current_width_offset}") + self.trigger_processing(self.files[self.index], self.current_shift) + def on_next(self, _event: object) -> None: if self.is_processing or self.current_result is None: return @@ -317,6 +340,7 @@ class ImageReviewer: def _advance(self) -> None: self.index += 1 self.current_shift = 0 + self.current_width_offset = 0 self.current_max_per_file = self.default_max_per_file self.load_current_image() diff --git a/copienator/commands/plotting.py b/copienator/commands/plotting.py index 1696173..93f39bb 100644 --- a/copienator/commands/plotting.py +++ b/copienator/commands/plotting.py @@ -1,7 +1,9 @@ from __future__ import annotations import argparse +import copy import queue +import re import threading import tkinter as tk from collections.abc import Sequence @@ -24,6 +26,9 @@ from copienator.utils import natural_key, read_all_labels # --- Configuration & Globals --- padding = 60 +MISSING_LABEL_COLOR = "orange" +COMMON_MISSING_LABEL_COLOR = "#403a00" +COMMON_MISSING_THRESHOLD = 0.66 try: font = ImageFont.truetype("DejaVuSans.ttf", size=30) @@ -81,13 +86,81 @@ def normalized_labels(entries): if str(value["label"]) != "_" ] -def prepare_image(image_path: str, bounding_boxes, all_labels, nb_pages, last_label_index): + +def frequently_missing_labels( + copies_dir: Path, + all_labels: list[str], + threshold: float = COMMON_MISSING_THRESHOLD, +) -> set[str]: + """Return labels absent from at least ``threshold`` of detected copies.""" + labels_by_copy: dict[str, set[str]] = {} + for json_path in copies_dir.glob("*.json"): + match = re.fullmatch(r"(.+)_\d+", json_path.stem) + if match is None: + continue + try: + data = read_json(json_path) + entries = data["list"] + if not isinstance(entries, list): + continue + present = set(normalized_labels(entries)) + except (OSError, KeyError, TypeError, ValueError): + continue + labels_by_copy.setdefault(match.group(1), set()).update(present) + + copy_count = len(labels_by_copy) + if copy_count == 0: + return set() + return { + label + for label in all_labels + if sum(label not in present for present in labels_by_copy.values()) + >= threshold * copy_count + } + + +def label_color( + label: str | None, + all_labels: list[str], + last_label_index: int, + common_missing: set[str], +) -> tuple[str, int]: + """Choose a label color and return the updated chronological index.""" + color = "black" + if not label or label not in all_labels: + return color, last_label_index + + current_index = all_labels.index(label) + if current_index < last_label_index or ( + last_label_index == -1 and current_index != 0 + ): + color = "red" + elif current_index > last_label_index + 1: + only_previous_is_missing = current_index == last_label_index + 2 + previous_label = all_labels[current_index - 1] + color = ( + COMMON_MISSING_LABEL_COLOR + if only_previous_is_missing and previous_label in common_missing + else MISSING_LABEL_COLOR + ) + return color, current_index + + +def prepare_image( + image_path: str, + bounding_boxes, + all_labels, + nb_pages, + last_label_index, + common_missing: set[str] | None = None, +): im = Image.open(image_path) im.load() width, height = im.size new_im = Image.new(im.mode, (width + padding, height), "white") new_im.paste(im, (0, 0)) draw = ImageDraw.Draw(new_im) + common_missing = common_missing or set() for bbox in sort_bounding_boxes(bounding_boxes, nb_pages): raw_y_min = int(bbox["box_2d"][0] * height / 1000) @@ -99,15 +172,13 @@ def prepare_image(image_path: str, bounding_boxes, all_labels, nb_pages, last_la abs_y_max = min(height, raw_y_max + 10) abs_x_max = min(width, raw_x_max + 10) - color = "black" label = bbox.get("label") - if label and label in all_labels: - current_index = all_labels.index(label) - if current_index < last_label_index or (last_label_index == -1 and current_index != 0): - color = "red" - elif current_index > last_label_index + 1: - color = "orange" - last_label_index = current_index + color, last_label_index = label_color( + label, + all_labels, + last_label_index, + common_missing, + ) draw.rectangle(((abs_x_min, abs_y_min), (abs_x_max, abs_y_max)), outline=color, width=4) if label: @@ -126,6 +197,7 @@ def _worker_items(base_dir, files_to_process, all_labels, output_queue): """ previous_copie = None last_label_index = None + common_missing = frequently_missing_labels(base_dir / "Copies", all_labels) for img_path in files_to_process: json_path = base_dir / "Copies" / f"{img_path.stem}.json" copie_part = int(img_path.stem[-2:]) @@ -158,7 +230,14 @@ def _worker_items(base_dir, files_to_process, all_labels, output_queue): try: print(f"Buffering {img_path.name}...") (pil_image, last_label_index) = \ - prepare_image(str(img_path), bb_list, all_labels, nb_pages, last_label_index) + prepare_image( + str(img_path), + bb_list, + all_labels, + nb_pages, + last_label_index, + common_missing, + ) error_msg = None except Exception as e: # noqa: BLE001 - keep the item editable in the GUI @@ -271,7 +350,6 @@ class ImageViewer: # Start new batch self.active_copie_name = metadata["copie"] self.accumulated_results = {"name": metadata["name"], "list": []} - self.history.clear() self.display_image(pil_image, json_path, metadata) except queue.Empty: @@ -293,16 +371,23 @@ class ImageViewer: def on_previous(self, event): if self.is_viewing and self.history: print("Going back to previous image...") - prev_pil, prev_json, prev_meta, num_added = self.history.pop() - - # Undo the accumulation to prevent duplicates when we hit Enter again - if self.accumulated_results and num_added > 0: - self.accumulated_results["list"] = self.accumulated_results["list"][:-num_added] + ( + prev_pil, + prev_json, + prev_meta, + previous_copie_name, + previous_results, + ) = self.history.pop() # Push current image to the forward stack so we don't lose it self.forward_stack.append((self.current_pil_image, self.current_json_path, self.current_meta)) + # Restore the aggregation exactly as it was before validating the + # previous image. This also makes navigation across copies safe. + self.active_copie_name = previous_copie_name + self.accumulated_results = previous_results + # Display the previous image immediately self.display_image(prev_pil, prev_json, prev_meta) def display_image(self, pil_image, json_path, metadata): @@ -329,7 +414,8 @@ class ImageViewer: def on_enter(self, event): if self.is_viewing: print(f"Committing data for {self.current_json_path.name}...") - num_added = 0 # ADD THIS LINE + previous_copie_name = self.active_copie_name + previous_results = copy.deepcopy(self.accumulated_results) try: current_data = read_json(self.current_json_path) @@ -352,8 +438,6 @@ class ImageViewer: print(msg) messagebox.showerror("Label Error", msg) return - num_added = len(converted_items) - # Add to accumulator if self.accumulated_results: self.accumulated_results["list"].extend(converted_items) @@ -368,8 +452,15 @@ class ImageViewer: messagebox.showerror("JSON Error", msg) return # Abort advancement - self.history.append((self.current_pil_image, self.current_json_path, - self.current_meta, num_added)) + self.history.append( + ( + self.current_pil_image, + self.current_json_path, + self.current_meta, + previous_copie_name, + previous_results, + ) + ) # Advance UI self.is_viewing = False diff --git a/copienator/workspace.py b/copienator/workspace.py index c6245a9..aadc914 100644 --- a/copienator/workspace.py +++ b/copienator/workspace.py @@ -109,6 +109,10 @@ class EvaluationWorkspace: def correction_progress_file(self) -> Path: return self.root / "correction_progress.json" + @property + def correction_pending_responses_file(self) -> Path: + return self.root / "correction_pending_responses.json" + @property def batch_jobs_file(self) -> Path: return self.root / "batch_jobs.json" diff --git a/copienator_gui/app.py b/copienator_gui/app.py index 92f62c4..d7e2a81 100644 --- a/copienator_gui/app.py +++ b/copienator_gui/app.py @@ -22,6 +22,7 @@ from copienator.platform import ( from .diagnostics import collect_diagnostics from .batch_monitor import BatchMonitor +from .giving_names import GivingNamesPanel, find_name_issues from .notifications import notify_desktop from .manual_resolution import ManualResolutionPanel from .refaire import SECTION as REFAIRE_SECTION @@ -58,6 +59,9 @@ STATUS_LABELS = { } DEFAULT_HTTPS_PROXY = "http://10.0.0.1:3128" +CORRECTION_PROGRESS_RE = re.compile( + r"\[Progression correction\] Groupes traités : (\d+)/(\d+)" +) PERSONAL_INTERRO_SOURCE = Path("/home/sebastien/Prépa/Staging/Interro") ANNOTATION_DIRECTORIES = ("BGnot", "Bnot", "Anot") ANNOTATION_VARIANT_DIRECTORIES = { @@ -65,6 +69,10 @@ ANNOTATION_VARIANT_DIRECTORIES = { "checks": "Bnot", "grouped": "BGnot", } +ANNOTATION_VARIANT_READERS = { + "checks": "standard", + "grouped": "grouped", +} def get_personal_interro_files( @@ -259,8 +267,11 @@ class CopienatorApp(tk.Tk): self.copy_paths: dict[str, Path] = {} self._rendering = False self.refaire_panel: RefaireSelection | None = None + self.giving_names_panel: GivingNamesPanel | None = None + self.complete_giving_names_button: ttk.Button | None = None self.pending_refaire_commands: list[list[str]] = [] self.refaire_command_index = 0 + self._correction_progress_buffer = "" self.title("Copienator — assistant de correction") self.geometry("1180x820") @@ -752,7 +763,15 @@ class CopienatorApp(tk.Tk): return self._rendering = True redo = step.section == REFAIRE_SECTION - expanded_form = redo or step.id in {"page_splitter", "crop_blank_margins", "cutleft", "labels", "manual_resolution"} + expanded_form = redo or step.id in { + "page_splitter", + "crop_blank_margins", + "cutleft", + "labels", + "plotting", + "manual_resolution", + "giving_names", + } self.console.configure(height=8 if expanded_form else 14) self.rowconfigure(1, weight=4 if expanded_form else 3) self.rowconfigure(2, weight=1 if expanded_form else 2) @@ -763,6 +782,8 @@ class CopienatorApp(tk.Tk): child.destroy() self.arg_vars.clear() self.refaire_panel = None + self.giving_names_panel = None + self.complete_giving_names_button = None entry = self.state_store.step(step.id) if self.state_store.evaluation else {} saved_variant = entry.get("variant", step.variants[0].id) if saved_variant not in {variant.id for variant in step.variants}: @@ -909,6 +930,31 @@ class CopienatorApp(tk.Tk): ) row += 1 return row + if step.id == "giving_names" and self.evaluation: + self.giving_names_panel = GivingNamesPanel( + self.form, self.evaluation + ) + self.giving_names_panel.grid( + row=row, + column=0, + columnspan=2, + sticky="ew", + pady=(0, 8), + ) + row += 1 + self.complete_giving_names_button = ttk.Button( + self.form, + text="Marquer terminée", + command=self._complete_giving_names, + ) + self.complete_giving_names_button.grid( + row=row, + column=0, + columnspan=2, + sticky="w", + pady=(0, 8), + ) + row += 1 if step.section == REFAIRE_SECTION: evaluation = self.evaluation if step.id in {"refaire_selection", "refaire_correct"}: @@ -935,7 +981,14 @@ class CopienatorApp(tk.Tk): return row + 1 if step.id in {"review_persp", "correction"}: row = self._correction_folder_buttons(row) - if step.id in {"page_splitter", "crop_blank_margins", "cutleft", "labels"}: + copy_selection_steps = { + "page_splitter", + "crop_blank_margins", + "cutleft", + "labels", + "plotting", + } + if step.id in copy_selection_steps: evaluation = self.evaluation paths = copy_pdf_paths(evaluation) if evaluation else [] self.copy_paths = {path.name: path for path in paths} @@ -966,7 +1019,7 @@ class CopienatorApp(tk.Tk): command=self._open_selected_copy, state="normal" if names else "disabled", ).grid(row=1, column=1, padx=(6, 0), pady=(7, 0)) - if step.id in {"page_splitter", "crop_blank_margins", "cutleft", "labels"}: + if step.id in copy_selection_steps: actions = ttk.Frame(copies) actions.grid(row=2, column=0, columnspan=2, sticky="w", pady=(7, 0)) label = "Refaire la copie sélectionnée" if step.id == "page_splitter" else "Cibler la copie sélectionnée" @@ -978,6 +1031,8 @@ class CopienatorApp(tk.Tk): if step.id == "page_splitter" else "Seule la copie ciblée sera analysée par Gemini. " if step.id == "labels" + else "Seule la copie ciblée sera vérifiée. " + if step.id == "plotting" else "Seule la copie ciblée sera traitée. " ) ttk.Label(copies, text=help_text + "Vérifiez la commande puis cliquez sur Exécuter.", @@ -1352,8 +1407,7 @@ class CopienatorApp(tk.Tk): return if step.is_manual: self._mark_step("success") - if step.section == REFAIRE_SECTION: - self._move_selection_from(step.id, 1) + self._move_selection_from(step.id, 1) return if os.name == "nt" and step.id != "statement": labels_path = evaluation / "labels" @@ -1400,6 +1454,9 @@ class CopienatorApp(tk.Tk): last_run_values=run_values, ) self.active_step_id = step.id + if step.id == "correction": + self._correction_progress_buffer = "" + self.info_var.set("Correction : préparation des groupes…") workspace = self.state_store.workspace assert workspace is not None log_path = workspace.log_path(step.id) @@ -1463,6 +1520,33 @@ class CopienatorApp(tk.Tk): self._populate_tree() self.info_var.set("Renommage validé — aucun fichier modifié.") + def _complete_giving_names(self) -> None: + if ( + not self.current_step + or self.current_step.id != "giving_names" + or not self.evaluation + or self.active_step_id + or self.runner.running + ): + return + return_dir = EvaluationWorkspace(self.evaluation).return_dir + if not return_dir.is_dir(): + messagebox.showerror( + "A Rendre absent", + "Exécutez d’abord l’attribution des noms.", + ) + return + issues = find_name_issues(self.evaluation) + if issues and not messagebox.askyesno( + "Noms encore à vérifier", + f"{len(issues)} copie(s) ont encore un nom inconnu ou dupliqué. " + "Marquer quand même l’étape comme terminée ?", + icon="warning", + ): + return + self._mark_step("success") + self._move_selection_from("giving_names", 1) + def _skip_step(self) -> None: if self.current_step and self.current_step.optional: step_id = self.current_step.id @@ -1477,7 +1561,9 @@ class CopienatorApp(tk.Tk): except queue.Empty: break if event == "output": - self._append_console(str(payload)) + output = str(payload) + self._track_correction_progress(output) + self._append_console(output) elif event == "input_echo": self._append_console(f"> {payload}") elif event == "runner_error": @@ -1487,6 +1573,24 @@ class CopienatorApp(tk.Tk): self._finish_process(int(return_code), bool(interrupted)) self.after(60, self._poll_runner) + def _track_correction_progress(self, output: str) -> None: + if self.active_step_id != "correction": + return + self._correction_progress_buffer = ( + self._correction_progress_buffer + output + )[-512:] + matches = list(CORRECTION_PROGRESS_RE.finditer( + self._correction_progress_buffer + )) + if not matches: + return + completed, total = (int(value) for value in matches[-1].groups()) + percent = round(100 * completed / total) if total else 100 + self.info_var.set( + f"Correction : {completed} groupe(s) traité(s) sur {total} " + f"({percent} %)." + ) + def _batch_watch_status_changed(self, message: str) -> None: self.batch_watch_status.set(message) self._update_controls() @@ -1565,7 +1669,25 @@ class CopienatorApp(tk.Tk): "été conservés.", ) return - self.state_store.update_step(step_id, status=status, return_code=return_code) + stored_status = ( + "detected" + if step_id == "giving_names" and status == "success" + else status + ) + self.state_store.update_step( + step_id, status=stored_status, return_code=return_code + ) + if step_id == "annotation" and status == "success": + generation_variant = self.state_store.step("annotation").get( + "last_run_variant" + ) + reader_variant = ANNOTATION_VARIANT_READERS.get( + str(generation_variant) + ) + if reader_variant: + self.state_store.update_step( + "read_annotations", variant=reader_variant + ) self.state_store.add_history( { "step": step_id, @@ -1580,12 +1702,27 @@ class CopienatorApp(tk.Tk): self.active_step_id = None self._populate_tree() self._update_controls() + if step_id == "giving_names" and status == "success": + if self.giving_names_panel: + self.giving_names_panel.reload() + self.info_var.set( + "A Rendre a été préparé. Vérifiez les noms, puis cliquez " + "sur « Marquer terminée »." + ) + return if status == "success": if step_id == "batch_status" and self.batch_monitor.active: self.batch_monitor.stop() self._batch_results_ready() return - self.after_idle(lambda completed_id=step_id: self._move_selection_from(completed_id, 1)) + if step_id == "manual_resolution": + self.after_idle(self._select_refaire_correction) + else: + self.after_idle( + lambda completed_id=step_id: self._move_selection_from( + completed_id, 1 + ) + ) def _send_input(self) -> None: text = self.stdin_var.get() @@ -1597,12 +1734,21 @@ class CopienatorApp(tk.Tk): self.stdin_var.set("") def _interrupt(self) -> None: - if self.runner.running and messagebox.askyesno( - "Interrompre", "Envoyer une interruption au script en cours ?" - ): + graceful_correction = self.active_step_id == "correction" + question = ( + "Arrêter de lancer de nouveaux appels Gemini ? Les appels déjà en " + "cours pourront se terminer et leurs résultats seront sauvegardés." + if graceful_correction + else "Envoyer une interruption au script en cours ?" + ) + if self.runner.running and messagebox.askyesno("Interrompre", question): try: self.runner.interrupt() - self.info_var.set("Interruption demandée. Utilisez « Forcer l’arrêt » si le script ne répond pas.") + self.info_var.set( + "Arrêt demandé : attente des appels Gemini en cours…" + if graceful_correction + else "Interruption demandée. Utilisez « Forcer l’arrêt » si le script ne répond pas." + ) except OSError as exc: messagebox.showerror("Interruption impossible", str(exc)) @@ -1645,6 +1791,10 @@ class CopienatorApp(tk.Tk): self.force_button.configure(state="normal" if running else "disabled") self.send_button.configure(state="normal" if running else "disabled") self.stdin_entry.configure(state="normal" if running else "disabled") + if self.complete_giving_names_button is not None: + self.complete_giving_names_button.configure( + state="disabled" if running else "normal" + ) def _move_selection(self, delta: int) -> None: if not self.current_step: @@ -1663,6 +1813,14 @@ class CopienatorApp(tk.Tk): self.tree.selection_set(ids[target]) self.tree.see(ids[target]) + def _select_refaire_correction(self) -> None: + """Return from manual conflict resolution to the main redo correction.""" + if self.runner.running or self.active_step_id or not self.state_store.evaluation: + return + self.state_store.update_step("correction", variant="refaire") + self.tree.selection_set("correction") + self.tree.see("correction") + def _on_close(self) -> None: if self.runner.running: if not messagebox.askyesno( diff --git a/copienator_gui/workflow.py b/copienator_gui/workflow.py index 6a19e74..c54a4f3 100644 --- a/copienator_gui/workflow.py +++ b/copienator_gui/workflow.py @@ -259,8 +259,11 @@ def build_workflow(show_personal_steps: bool) -> list[StepDefinition]: StepDefinition( "cutleft", "Prétraitement des copies", - "Découper la marge des labels", - "Produit les images de la partie gauche des copies. Une copie précise peut être ciblée.", + "Découper une partie à gauche pour détection des labels", + "Produit les images de la partie gauche des copies. Dans la fenêtre de découpe : " + "n décale la zone de 50 px vers la droite, N de 100 px, t de 50 px vers la gauche, " + "l l’élargit de 50 px, 1 utilise les pages entières, Entrée valide et s signale une erreur. " + "Une copie précise peut être ciblée.", (python("default", "Découpe", "crop-labels"),), arguments=( arg_target(), @@ -309,7 +312,8 @@ def build_workflow(show_personal_steps: bool) -> list[StepDefinition]: "plotting", "Labels et regroupement", "Vérifier visuellement les labels", - "Ouvre la fenêtre de vérification. La cible peut être toute l’évaluation ou une copie.", + "Ouvre la fenêtre de vérification. La cible peut être toute l’évaluation ou une copie. " + "Le raccourci p revient à l’image précédente, y compris dans la copie précédente.", (python("default", "Vérification", "review-labels"),), arguments=(arg_target(),), requires=("labels", "Cutleft"), @@ -364,7 +368,10 @@ def build_workflow(show_personal_steps: bool) -> list[StepDefinition]: "correction", "Correction", "Lancer ou intégrer la correction", - "Choisir une correction immédiate, batch, hybride, une recorrection, ou l’intégration d’un batch.", + "Choisir une correction immédiate, batch, hybride, une recorrection, ou l’intégration d’un batch. " + "En correction immédiate, la barre d’état affiche le nombre de groupes traités et le total. " + "Interrompre bloque les nouveaux appels Gemini, attend ceux déjà en cours, sauvegarde leurs résultats puis arrête la commande. " + "Au prochain lancement, les validations encore nécessaires (mauvais label ou contenu supplémentaire) reprennent sans refaire la requête principale.", ( python("live", "Correction immédiate", "correct"), python("batch", "Préparer toutes les requêtes batch", "correct", fixed_args=("--batch",)), @@ -386,7 +393,7 @@ def build_workflow(show_personal_steps: bool) -> list[StepDefinition]: "bool", "--overwrite", help="Relance les corrections demandées même lorsqu’un résultat existe déjà.", - variants=("live", "batch", "hybrid", "refaire", "integrate"), + variants=("live", "batch", "hybrid", "integrate"), ), ArgumentSpec( "limit", @@ -443,6 +450,18 @@ def build_workflow(show_personal_steps: bool) -> list[StepDefinition]: optional=True, skip_for_live_correction=True, ), + StepDefinition( + "manual_resolution", + "Correction", + "Résoudre les conflits manuels", + "Étape conditionnelle, uniquement si manual_resolutions.txt contient des conflits à traiter. " + "Après succès, le GUI revient à la correction avec « Recorrection depuis refaire.json » sélectionné.", + (python("default", "Résolution", "resolve-manual"),), + arguments=(arg_target("le dossier de l’évaluation"),), + optional=True, + requires=("manual_resolutions.txt", "correction.json"), + skip_without_manual_conflicts=True, + ), StepDefinition( "post_correction", "Correction", @@ -452,17 +471,6 @@ def build_workflow(show_personal_steps: bool) -> list[StepDefinition]: arguments=(arg_target("le dossier de l’évaluation"),), requires=("correction.json",), ), - StepDefinition( - "manual_resolution", - "Correction", - "Résoudre les conflits manuels", - "Étape conditionnelle, uniquement si manual_resolutions.txt contient des conflits à traiter.", - (python("default", "Résolution", "resolve-manual"),), - arguments=(arg_target("le dossier de l’évaluation"),), - optional=True, - requires=("manual_resolutions.txt", "correction.json"), - skip_without_manual_conflicts=True, - ), StepDefinition( "annotation", "Génération des annotations", @@ -596,7 +604,9 @@ def build_workflow(show_personal_steps: bool) -> list[StepDefinition]: "giving_names", "Finalisation", "Attribuer les noms et préparer A Rendre", - "Crée le dossier A Rendre à partir du dossier d’annotations choisi.", + "Crée le dossier A Rendre à partir du dossier d’annotations choisi, sans passer automatiquement à l’étape suivante. " + "Après l’exécution, vous pouvez renommer chaque dossier, ainsi que son fichier .jpg et son fichier .pdf, avec un autre nom ; le suffixe (id) du dossier est conservé. " + "Les outils affichés permettent d’identifier et corriger les noms Unknown ou attribués à plusieurs copies. Cliquez ensuite sur « Marquer terminée ».", (python("default", "Attribution des noms", "giving-names"),), arguments=( arg_target("le dossier de l’évaluation"), diff --git a/tests/test_gui_convenience.py b/tests/test_gui_convenience.py index 8d70d18..14cbfba 100644 --- a/tests/test_gui_convenience.py +++ b/tests/test_gui_convenience.py @@ -85,6 +85,53 @@ class GuiConvenienceTests(unittest.TestCase): self.app.manual_panel.editor_button.invoke() opened.assert_called_once_with(other_manual) + def test_successful_manual_resolution_returns_to_refaire_correction(self): + manual = self.evaluation / "manual_resolutions.txt" + manual.write_text("Copie01 A -> B|\n", encoding="utf-8") + self.app.state_store.update_step("manual_resolution", visited=True) + self.app.tree.selection_set("manual_resolution") + self.app.update() + + self.app.active_step_id = "manual_resolution" + self.app._finish_process(0, False) + self.app.update() + + self.assertEqual(self.app.current_step.id, "correction") + self.assertEqual(self.app.variant_var.get(), "refaire") + self.assertEqual( + self.app.state_store.step("correction")["variant"], "refaire" + ) + self.assertNotIn("overwrite", self.app.arg_vars) + self.assertIn("--refaire", self.app._make_command()) + + def test_giving_names_waits_for_explicit_manual_completion(self): + return_dir = self.evaluation / "A Rendre" / "Student (01)" + return_dir.mkdir(parents=True) + (return_dir / "Student.jpg").write_bytes(b"jpg") + (return_dir / "Student.pdf").write_bytes(b"pdf") + self.app.tree.selection_set("giving_names") + self.app.update() + + self.app.active_step_id = "giving_names" + self.app._finish_process(0, False) + self.app.update() + + self.assertEqual(self.app.current_step.id, "giving_names") + self.assertEqual( + self.app.state_store.step("giving_names")["status"], "detected" + ) + self.assertEqual( + self.app.complete_giving_names_button.cget("text"), + "Marquer terminée", + ) + + self.app.complete_giving_names_button.invoke() + self.app.update() + self.assertEqual( + self.app.state_store.step("giving_names")["status"], "success" + ) + self.assertNotEqual(self.app.current_step.id, "giving_names") + def test_batch_status_waits_until_all_jobs_are_ready(self): self.app.state_store.update_step("correction", variant="batch") self.app.state_store.update_step("batch_status", visited=True) @@ -213,6 +260,28 @@ class GuiConvenienceTests(unittest.TestCase): self.assertIn("Cibler la copie sélectionnée", controls) self.assertIn("Cibler tout le dossier", controls) + def test_label_review_can_show_and_target_one_copy(self): + copies = self.evaluation / "Copies" + copies.mkdir() + for name in ("Copie01.pdf", "Copie02.pdf"): + (copies / name).touch() + + self.app.tree.selection_set("plotting") + self.app.update() + self.app.copy_var.set("Copie02.pdf") + + with patch("copienator_gui.app.open_path") as opened: + self.app._open_selected_copy() + opened.assert_called_once_with(copies / "Copie02.pdf") + + self.app._target_selected_copy() + command = self.app._make_command() + self.assertEqual(Path(command[-1]), copies / "Copie02.pdf") + labels = self._label_texts(self.app.form) + self.assertTrue( + any("Seule la copie ciblée sera vérifiée" in text for text in labels) + ) + def test_free_form_arguments_are_shown_only_when_documented(self): self.app.tree.selection_set("labels") self.app.update() diff --git a/tests/test_gui_core.py b/tests/test_gui_core.py index e03926b..98128a5 100644 --- a/tests/test_gui_core.py +++ b/tests/test_gui_core.py @@ -43,6 +43,11 @@ from copienator_gui.app import ( plotting_shortcut_lines, process_status, ) +from copienator_gui.giving_names import ( + find_name_issues, + original_copy_path, + rename_return_copy, +) from copienator_gui.diagnostics import collect_diagnostics from copienator_gui.runner import ProcessRunner from copienator_gui.state import StateStore @@ -911,6 +916,43 @@ class StandardCliTests(unittest.TestCase): ) self.assertIsNone(viewer.accumulated_results) + def test_plotting_detects_labels_missing_from_two_thirds_of_copies(self) -> None: + module = self.modules["plotting"] + with tempfile.TemporaryDirectory() as directory: + copies = Path(directory) + detected = { + "Copie01_01.json": ["Ex 1", "Ex 3"], + "Copie02_01.json": ["Ex 1", "Ex 3"], + "Copie03_01.json": ["Ex 1", "Ex 2", "Ex 3"], + } + for filename, labels in detected.items(): + atomic_write_json( + copies / filename, + {"list": [{"label": label} for label in labels]}, + ) + + missing = module.frequently_missing_labels( + copies, + ["Ex 1", "Ex 2", "Ex 3", "Ex 4"], + ) + + self.assertEqual(missing, {"Ex 2", "Ex 4"}) + + def test_plotting_mutes_only_a_single_commonly_missing_label(self) -> None: + module = self.modules["plotting"] + labels = ["Ex 1", "Ex 2", "Ex 3", "Ex 4"] + + muted, index = module.label_color("Ex 3", labels, 0, {"Ex 2"}) + ordinary, _ = module.label_color("Ex 3", labels, 0, set()) + two_missing, _ = module.label_color( + "Ex 4", labels, 0, {"Ex 2", "Ex 3"} + ) + + self.assertEqual(muted, module.COMMON_MISSING_LABEL_COLOR) + self.assertEqual(index, 2) + self.assertEqual(ordinary, module.MISSING_LABEL_COLOR) + self.assertEqual(two_missing, module.MISSING_LABEL_COLOR) + def test_plotting_validation_preserves_manually_edited_order(self) -> None: module = self.modules["plotting"] with tempfile.TemporaryDirectory() as directory: @@ -931,6 +973,7 @@ class StandardCliTests(unittest.TestCase): "part": 1, } viewer.valid_labels = {"Ex 1", "Ex 2"} + viewer.active_copie_name = "Copie01" viewer.accumulated_results = {"name": "Student", "list": []} viewer.history = [] viewer.current_pil_image = Mock() @@ -944,6 +987,49 @@ class StandardCliTests(unittest.TestCase): ["Ex 2", "Ex 1"], ) + def test_plotting_previous_restores_state_across_copies(self) -> None: + module = self.modules["plotting"] + viewer = module.ImageViewer.__new__(module.ImageViewer) + previous_image = Mock() + previous_json = Path("Copie01_02.json") + previous_meta = {"copie": "Copie01"} + previous_results = { + "name": "Student 1", + "list": [{"label": "Ex 1"}], + } + current_image = Mock() + current_json = Path("Copie02_01.json") + current_meta = {"copie": "Copie02"} + viewer.is_viewing = True + viewer.history = [ + ( + previous_image, + previous_json, + previous_meta, + "Copie01", + previous_results, + ) + ] + viewer.forward_stack = [] + viewer.current_pil_image = current_image + viewer.current_json_path = current_json + viewer.current_meta = current_meta + viewer.active_copie_name = "Copie02" + viewer.accumulated_results = {"name": "Student 2", "list": []} + viewer.display_image = Mock() + + viewer.on_previous(None) + + self.assertEqual(viewer.active_copie_name, "Copie01") + self.assertEqual(viewer.accumulated_results, previous_results) + self.assertEqual( + viewer.forward_stack, + [(current_image, current_json, current_meta)], + ) + viewer.display_image.assert_called_once_with( + previous_image, previous_json, previous_meta + ) + def test_plotting_validates_plain_and_directional_labels(self) -> None: module = self.modules["plotting"] self.assertEqual( @@ -1226,6 +1312,27 @@ class StandardCliTests(unittest.TestCase): self.assertEqual(module.results, {"Ex 1": []}) self.assertEqual(module.tasks_to_process, [("new.jpg", "Ex 1")]) + def test_correction_rejects_overwrite_with_refaire_without_writing(self) -> None: + module = self.modules["correction"] + with tempfile.TemporaryDirectory() as directory: + evaluation = Path(directory) / "Exam" + evaluation.mkdir() + correction = evaluation / "correction.json" + original = {"Ex 1": [[{"id": "01", "result": {"feedback": []}}]]} + atomic_write_json(correction, original) + + errors = io.StringIO() + with redirect_stderr(errors): + status = module.main( + [str(evaluation), "--refaire", "--overwrite"] + ) + + self.assertEqual(status, module.ExitCode.INVALID_ARGUMENTS) + self.assertIn( + "--overwrite cannot be used with --refaire", errors.getvalue() + ) + self.assertEqual(read_json(correction), original) + def test_correction_reserves_unique_group_indices_concurrently(self) -> None: module = self.modules["correction"] with tempfile.TemporaryDirectory() as directory: @@ -1247,6 +1354,251 @@ class StandardCliTests(unittest.TestCase): self.assertEqual(sorted(indices), list(range(3, 11))) self.assertEqual(len(indices), len(set(indices))) + def test_live_correction_reports_progress_for_generated_groups(self) -> None: + module = self.modules["correction"] + with tempfile.TemporaryDirectory() as directory: + workspace = EvaluationWorkspace(Path(directory)) + args = module.build_parser().parse_args( + [str(workspace.root), "--overwrite"] + ) + initial_tasks = [("group-1.jpg", "Ex 1"), ("group-2.jpg", "Ex 2")] + module.configure_runtime( + workspace, + initial_tasks, + args, + api_client=Mock(), + ) + processed = [] + + def process(task, _precomputed=None): + processed.append(task[0]) + if task[0] == "group-1.jpg": + return [("group-3.jpg", "Ex 3", False)] + return [] + + output = io.StringIO() + with patch.object( + module, "process_single_task", side_effect=process + ), patch.object( + module, "resolve_delayed_moves", return_value=[] + ), redirect_stdout(output): + self.assertEqual(module.run_configured(args), 0) + + self.assertCountEqual( + processed, + ["group-1.jpg", "group-2.jpg", "group-3.jpg"], + ) + progress = [ + line + for line in output.getvalue().splitlines() + if line.startswith("[Progression correction]") + ] + self.assertEqual(progress[0], "[Progression correction] Groupes traités : 0/2") + self.assertEqual(progress[-1], "[Progression correction] Groupes traités : 3/3") + + def test_live_correction_stops_scheduling_and_waits_for_active_task(self) -> None: + module = self.modules["correction"] + self.addCleanup(module.stop_requested.clear) + with tempfile.TemporaryDirectory() as directory: + workspace = EvaluationWorkspace(Path(directory)) + args = module.build_parser().parse_args( + [str(workspace.root), "--overwrite"] + ) + module.configure_runtime( + workspace, + [("group-1.jpg", "Ex 1"), ("group-2.jpg", "Ex 2")], + args, + api_client=Mock(), + ) + processed = [] + + def process(task, _precomputed=None): + processed.append(task[0]) + module.request_graceful_stop() + (workspace.root / "saved-result").write_text(task[0]) + return [] + + with patch.object(module, "NB_THREADS", 1), patch.object( + module, "process_single_task", side_effect=process + ), patch.object( + module, "resolve_delayed_moves", return_value=[] + ): + status = module.run_configured(args) + saved_result = (workspace.root / "saved-result").read_text() + + self.assertEqual(status, module.ExitCode.INTERRUPTED) + self.assertEqual(processed, ["group-1.jpg"]) + self.assertEqual(saved_result, "group-1.jpg") + + def test_gemini_call_finishes_inflight_stream_but_does_not_retry(self) -> None: + module = self.modules["correction"] + self.addCleanup(module.stop_requested.clear) + module.stop_requested.clear() + api_client = Mock() + + def stream(**_kwargs): + yield SimpleNamespace(text="first") + module.stop_requested.set() + yield SimpleNamespace(text="-second") + + api_client.models.generate_content_stream.side_effect = stream + with patch.object(module, "client", api_client): + self.assertEqual( + module.call_gemini_with_retries("model", [], Mock()), + "first-second", + ) + with self.assertRaises(module.CorrectionStopRequested): + module.call_gemini_with_retries("model", [], Mock()) + self.assertEqual(api_client.models.generate_content_stream.call_count, 1) + + def test_interrupted_live_correction_resumes_only_remaining_groups(self) -> None: + module = self.modules["correction"] + self.addCleanup(module.stop_requested.clear) + with tempfile.TemporaryDirectory() as directory: + workspace = EvaluationWorkspace(Path(directory)) + workspace.copies_dir.mkdir() + tasks = [] + for index, label in enumerate(("Ex 1", "Ex 2"), start=1): + group_dir = workspace.groups_dir / label + group_dir.mkdir(parents=True) + image = group_dir / f"Group_{index}.jpg" + image.write_bytes(b"image") + atomic_write_json( + image.with_suffix(".json"), + [["01", 0, 400, 1.0, label]], + ) + tasks.append((str(image), label)) + + response = json.dumps( + [{"id": "01", "result": {"error": "", "feedback": []}}] + ) + call_count = 0 + + def stream(**_kwargs): + nonlocal call_count + call_count += 1 + if call_count == 1: + module.stop_requested.set() + yield SimpleNamespace(text=response) + + api_client = Mock() + api_client.models.generate_content_stream.side_effect = stream + args = module.build_parser().parse_args([str(workspace.root)]) + + with patch.object(module, "NB_THREADS", 1), patch.object( + module.prompting, "generate_request", return_value=([], Mock()) + ): + module.configure_runtime( + workspace, tasks, args, api_client=api_client + ) + first_status = module.run_configured(args) + first_progress = read_json(workspace.correction_progress_file) + first_results = read_json(workspace.correction_file) + + module.configure_runtime( + workspace, tasks, args, api_client=api_client + ) + pending_on_resume = list(module.tasks_to_process) + second_status = module.run_configured(args) + + final_progress = read_json(workspace.correction_progress_file) + final_results = read_json(workspace.correction_file) + + self.assertEqual(first_status, module.ExitCode.INTERRUPTED) + self.assertEqual(first_progress, [list(tasks[0])]) + self.assertEqual(len(first_results["Ex 1"]), 1) + self.assertEqual(first_results["Ex 2"], []) + self.assertEqual(pending_on_resume, [tasks[1]]) + self.assertEqual(second_status, module.ExitCode.SUCCESS) + self.assertEqual(final_progress, [list(tasks[0]), list(tasks[1])]) + self.assertEqual(len(final_results["Ex 1"]), 1) + self.assertEqual(len(final_results["Ex 2"]), 1) + self.assertEqual(call_count, 2) + + def test_interrupted_label_errors_resume_auxiliary_requests(self) -> None: + module = self.modules["correction"] + self.addCleanup(module.stop_requested.clear) + + for error_type in ("wrong-label", "additional-answer"): + with self.subTest(error_type=error_type), tempfile.TemporaryDirectory() as directory: + workspace = EvaluationWorkspace(Path(directory)) + workspace.copies_dir.mkdir() + group_dir = workspace.groups_dir / "Ex 1" + group_dir.mkdir(parents=True) + image = group_dir / "Group_1.jpg" + image.write_bytes(b"image") + atomic_write_json( + image.with_suffix(".json"), + [["01", 0, 400, 1.0, "Ex 1"]], + ) + task = (str(image), "Ex 1") + response = json.dumps( + [ + { + "id": "01", + "result": {"error": error_type, "feedback": []}, + } + ] + ) + primary_call_count = 0 + + def stream(**_kwargs): + nonlocal primary_call_count + primary_call_count += 1 + module.stop_requested.set() + yield SimpleNamespace(text=response) + + def resolve_error(_pid, _label, result, _pdf_path): + self.assertEqual(result["error"], error_type) + result["error"] = "" + return [] + + api_client = Mock() + api_client.models.generate_content_stream.side_effect = stream + args = module.build_parser().parse_args( + [str(workspace.root), "--overwrite"] + ) + + with patch.object(module, "NB_THREADS", 1), patch.object( + module.prompting, "generate_request", return_value=([], Mock()) + ), patch.object( + module, "handle_label_errors", side_effect=resolve_error + ) as handler: + module.configure_runtime( + workspace, [task], args, api_client=api_client + ) + first_status = module.run_configured(args) + saved_pending = read_json( + workspace.correction_pending_responses_file + ) + first_progress_exists = ( + workspace.correction_progress_file.exists() + ) + first_correction_exists = workspace.correction_file.exists() + + module.configure_runtime( + workspace, [task], args, api_client=api_client + ) + second_status = module.run_configured(args) + + self.assertEqual(first_status, module.ExitCode.INTERRUPTED) + self.assertFalse(first_progress_exists) + self.assertFalse(first_correction_exists) + self.assertEqual( + json.loads(saved_pending[str(image)]), json.loads(response) + ) + self.assertEqual(second_status, module.ExitCode.SUCCESS) + self.assertEqual(primary_call_count, 1) + handler.assert_called_once() + self.assertEqual( + read_json(workspace.correction_progress_file), [list(task)] + ) + corrected = read_json(workspace.correction_file) + self.assertEqual(corrected["Ex 1"][0][0]["result"]["error"], "") + self.assertEqual( + read_json(workspace.correction_pending_responses_file), {} + ) + def test_correction_reset_restores_old_and_deletes_new_files(self) -> None: module = self.modules["correction"] with tempfile.TemporaryDirectory() as directory: @@ -1255,6 +1607,9 @@ class StandardCliTests(unittest.TestCase): copy_dir.mkdir(parents=True) atomic_write_json(evaluation / "correction.json", {"old": True}) atomic_write_json(evaluation / "correction_progress.json", ["old"]) + atomic_write_json( + evaluation / "correction_pending_responses.json", {"group": "response"} + ) (copy_dir / "Ex 1.pdf").write_bytes(b"current") (copy_dir / "Ex 1_old.pdf").write_bytes(b"original") (copy_dir / "Ex 2_new.pdf").write_bytes(b"generated") @@ -1262,6 +1617,9 @@ class StandardCliTests(unittest.TestCase): self.assertEqual(module.main([str(evaluation), "--reset"]), 0) self.assertFalse((evaluation / "correction.json").exists()) self.assertFalse((evaluation / "correction_progress.json").exists()) + self.assertFalse( + (evaluation / "correction_pending_responses.json").exists() + ) self.assertEqual((copy_dir / "Ex 1.pdf").read_bytes(), b"original") self.assertFalse((copy_dir / "Ex 1_old.pdf").exists()) self.assertFalse((copy_dir / "Ex 2_new.pdf").exists()) @@ -1798,6 +2156,13 @@ class WorkflowTests(unittest.TestCase): ) self.assertEqual(skip_without_conflicts, {"manual_resolution"}) + def test_manual_resolution_precedes_post_correction(self) -> None: + ordered_ids = [step.id for step in build_workflow(False)] + self.assertLess( + ordered_ids.index("manual_resolution"), + ordered_ids.index("post_correction"), + ) + def test_crop_auto_start_follows_always_crop_configuration(self) -> None: for enabled in (False, True): with self.subTest(enabled=enabled), patch( @@ -1823,6 +2188,16 @@ class WorkflowTests(unittest.TestCase): def test_review_persp_has_shorter_title(self) -> None: self.assertEqual(self.steps["review_persp"].title, "Relire les barèmes") + def test_cutleft_title_and_shortcuts_are_documented(self) -> None: + step = self.steps["cutleft"] + self.assertEqual( + step.title, + "Découper une partie à gauche pour détection des labels", + ) + for shortcut in ("n", "N", "t", "l", "1", "Entrée", "s"): + with self.subTest(shortcut=shortcut): + self.assertIn(shortcut, step.description) + def test_export_has_shorter_title_and_annotation_argument(self) -> None: self.assertEqual(self.steps["export"].title, "Exporter") command = self.command( @@ -1857,6 +2232,47 @@ class WorkflowTests(unittest.TestCase): detected_annotation_directories(evaluation), ("Bnot", "Anot") ) + def test_giving_names_utilities_find_and_rename_problematic_copies(self) -> None: + with tempfile.TemporaryDirectory() as directory: + evaluation = Path(directory) + return_dir = evaluation / "A Rendre" + original_dir = evaluation / "Copies Originales" + return_dir.mkdir() + original_dir.mkdir() + (original_dir / "Copie01.pdf").write_bytes(b"original") + folders = { + "Unknown (01)": (b"unknown-jpg", b"unknown-pdf"), + "Dupont (02)": (b"two-jpg", b"two-pdf"), + "Dupont (03)": (b"three-jpg", b"three-pdf"), + "Unique (04)": (b"four-jpg", b"four-pdf"), + } + for folder_name, (jpg, pdf) in folders.items(): + folder = return_dir / folder_name + folder.mkdir() + base_name = folder_name.rsplit(" (", 1)[0] + (folder / f"{base_name}.jpg").write_bytes(jpg) + (folder / f"{base_name}.pdf").write_bytes(pdf) + (folder / "score.json").write_text("{}", encoding="utf-8") + (folder / "answers").mkdir() + + issues = find_name_issues(evaluation) + self.assertEqual([issue.copy_id for issue in issues], ["01", "02", "03"]) + self.assertEqual( + original_copy_path(evaluation, "01"), + original_dir / "Copie01.pdf", + ) + + renamed = rename_return_copy(return_dir / "Unknown (01)", "Alice") + self.assertEqual(renamed.name, "Alice (01)") + self.assertEqual((renamed / "Alice.jpg").read_bytes(), b"unknown-jpg") + self.assertEqual((renamed / "Alice.pdf").read_bytes(), b"unknown-pdf") + self.assertTrue((renamed / "score.json").is_file()) + self.assertTrue((renamed / "answers").is_dir()) + self.assertEqual( + [issue.copy_id for issue in find_name_issues(evaluation)], + ["02", "03"], + ) + def test_export_and_import_defaults_follow_previous_runs(self) -> None: with tempfile.TemporaryDirectory() as directory: evaluation = Path(directory) @@ -1883,10 +2299,27 @@ class WorkflowTests(unittest.TestCase): def test_plotting_shortcuts_describe_open_actions(self) -> None: rendered = "\n".join(plotting_shortcut_lines()) + self.assertIn("p : revenir à la précédente", rendered) self.assertIn("ouvrir l’énoncé", rendered) self.assertIn("ouvrir la copie traitée", rendered) self.assertIn("ouvrir la copie originale", rendered) + def test_gui_tracks_correction_progress_across_output_chunks(self) -> None: + app = object.__new__(CopienatorApp) + app.active_step_id = "correction" + app._correction_progress_buffer = "" + app.info_var = Mock() + + app._track_correction_progress( + "[Progression correction] Groupes trai" + ) + app.info_var.set.assert_not_called() + app._track_correction_progress("tés : 3/8\n") + + app.info_var.set.assert_called_once_with( + "Correction : 3 groupe(s) traité(s) sur 8 (38 %)." + ) + def test_proxy_is_opt_in_and_prefilled(self) -> None: base = {"HTTPS_PROXY": "http://system-proxy", "OTHER": "kept"} without_proxy = build_runner_environment( @@ -1922,6 +2355,21 @@ class WorkflowTests(unittest.TestCase): ) self.assertEqual(command[4], "correct") + def test_refaire_correction_does_not_offer_overwrite(self) -> None: + step = self.steps["correction"] + overwrite = next( + argument for argument in step.arguments if argument.name == "overwrite" + ) + self.assertNotIn("refaire", overwrite.variants) + + command = self.command( + "correction", + "refaire", + {"target": self.evaluation, "overwrite": True}, + ) + self.assertIn("--refaire", command) + self.assertNotIn("--overwrite", command) + def test_hybrid_correction_arguments(self) -> None: command = self.command( "correction", "hybrid", {"target": self.evaluation, "batch_from": "Ex 4"} diff --git a/tests/test_marked_copies.py b/tests/test_marked_copies.py index 1e745fa..eaee78f 100644 --- a/tests/test_marked_copies.py +++ b/tests/test_marked_copies.py @@ -36,6 +36,7 @@ class MarkedCopiesTests(unittest.TestCase): review.had_errors = False review.completed = False review.current_shift = 50 + review.current_width_offset = 0 review.default_max_per_file = 5 review.current_max_per_file = 1 review.root = Mock() @@ -70,6 +71,7 @@ class MarkedCopiesTests(unittest.TestCase): self.assertIn("Copie01.pdf", copy_errors(self.workspace)) self.assertEqual(review.index, 1) self.assertEqual(review.current_max_per_file, 5) + self.assertEqual(review.current_width_offset, 0) self.assertTrue(review.had_errors) review.load_current_image.assert_called_once_with() @@ -96,6 +98,30 @@ class MarkedCopiesTests(unittest.TestCase): self.assertTrue(review.stop_prefetch.is_set()) self.assertIn("Copie01.pdf", copy_errors(self.workspace)) + def test_enlarge_reprocesses_with_a_wider_selection(self): + review = self.reviewer() + review.trigger_processing = Mock() + + review.on_enlarge(cutleft.CROP_WIDTH_STEP) + + self.assertEqual(review.current_width_offset, 50) + review.trigger_processing.assert_called_once_with(self.files[0], 50) + + def test_process_single_pdf_enlarges_crop_and_caps_it_at_page_edge(self): + page = Image.new("RGB", (900, 300), "white") + with patch.object(cutleft, "get_pdf_pages", return_value=[page]): + regular = cutleft.process_single_pdf(self.files[0]) + enlarged = cutleft.process_single_pdf(self.files[0], width_offset=200) + capped = cutleft.process_single_pdf(self.files[0], width_offset=1000) + fullpage = cutleft.process_single_pdf( + self.files[0], max_per_file=1, width_offset=200 + ) + + self.assertEqual(regular[1][0].size, (300, 300)) + self.assertEqual(enlarged[1][0].size, (500, 300)) + self.assertEqual(capped[1][0].size, (800, 300)) + self.assertEqual(fullpage[1][0].size, (900, 300)) + def test_processing_blocks_skip_and_failed_conversion_is_flagged(self): review = self.reviewer() review.is_processing = True