feat: add ValidationCoordinator for cross-cutting validation
Co-authored-by: aider (openrouter/anthropic/claude-sonnet-4) <aider@aider.chat>
This commit is contained in:
@@ -293,40 +293,8 @@ class OCROptions(BaseModel):
|
||||
data['output_file'] = '/dev/null' # Placeholder
|
||||
return data
|
||||
|
||||
@model_validator(mode='after')
|
||||
def validate_exclusive_ocr_options(self):
|
||||
"""Ensure only one of force_ocr, skip_text, redo_ocr is set."""
|
||||
exclusive_options = sum(
|
||||
1 for opt in [self.force_ocr, self.skip_text, self.redo_ocr] if opt
|
||||
)
|
||||
if exclusive_options >= 2:
|
||||
raise ValueError("Choose only one of --force-ocr, --skip-text, --redo-ocr.")
|
||||
return self
|
||||
|
||||
@model_validator(mode='after')
|
||||
def validate_output_type_compatibility(self):
|
||||
"""Validate output type is compatible with output file."""
|
||||
if self.output_type == 'none' and str(self.output_file) not in (
|
||||
os.devnull,
|
||||
'-',
|
||||
):
|
||||
raise ValueError(
|
||||
"Since you specified `--output-type none`, the output file "
|
||||
f"{self.output_file} cannot be produced. Set the output file to "
|
||||
f"`-` to suppress this message."
|
||||
)
|
||||
return self
|
||||
|
||||
@model_validator(mode='after')
|
||||
def validate_redo_ocr_options(self):
|
||||
"""Validate options compatible with redo_ocr."""
|
||||
if self.redo_ocr:
|
||||
if self.deskew or self.clean_final or self.remove_background:
|
||||
raise ValueError(
|
||||
"--redo-ocr is not currently compatible with --deskew, "
|
||||
"--clean-final, and --remove-background"
|
||||
)
|
||||
return self
|
||||
# Note: Cross-cutting validation moved to ValidationCoordinator
|
||||
# Basic field validation remains here, complex validation moved to coordinator
|
||||
|
||||
@property
|
||||
def lossless_reconstruction(self):
|
||||
|
||||
@@ -121,12 +121,28 @@ def _check_plugin_invariant_options(options: OCROptions) -> None:
|
||||
|
||||
|
||||
def _check_plugin_options(options: OCROptions, plugin_manager: PluginManager) -> None:
|
||||
# First, let plugins check their external dependencies
|
||||
plugin_manager.hook.check_options(options=options)
|
||||
|
||||
# Then check OCR engine language support
|
||||
ocr_engine_languages = plugin_manager.hook.get_ocr_engine().languages(options)
|
||||
check_options_languages(options, ocr_engine_languages)
|
||||
|
||||
# Finally, run comprehensive validation using the coordinator
|
||||
from ocrmypdf._validation_coordinator import ValidationCoordinator
|
||||
coordinator = ValidationCoordinator(plugin_manager)
|
||||
coordinator.validate_all_options(options)
|
||||
|
||||
|
||||
def check_options(options: OCROptions, plugin_manager: PluginManager) -> None:
|
||||
"""Check options for validity and consistency.
|
||||
|
||||
This function coordinates validation across the entire system:
|
||||
1. Core validation (platform, files, preprocessing)
|
||||
2. Plugin external dependency validation
|
||||
3. Plugin-specific validation (handled by plugin models)
|
||||
4. Cross-cutting validation (handled by validation coordinator)
|
||||
"""
|
||||
_check_plugin_invariant_options(options)
|
||||
_check_plugin_options(options, plugin_manager)
|
||||
|
||||
|
||||
@@ -0,0 +1,131 @@
|
||||
# SPDX-FileCopyrightText: 2024 James R. Barlow
|
||||
# SPDX-License-Identifier: MPL-2.0
|
||||
|
||||
"""Validation coordinator for plugin options and cross-cutting concerns."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import os
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
if TYPE_CHECKING:
|
||||
import pluggy
|
||||
from ocrmypdf._options import OCROptions
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class ValidationCoordinator:
|
||||
"""Coordinates validation across plugin models and core options."""
|
||||
|
||||
def __init__(self, plugin_manager: pluggy.PluginManager):
|
||||
self.plugin_manager = plugin_manager
|
||||
self.registry = getattr(plugin_manager, '_option_registry', None)
|
||||
|
||||
def validate_all_options(self, options: OCROptions) -> None:
|
||||
"""Run comprehensive validation on all options.
|
||||
|
||||
This runs validation in the correct order:
|
||||
1. Plugin self-validation (already done by Pydantic)
|
||||
2. Plugin context validation (requires external context)
|
||||
3. Cross-cutting validation (between plugins and core)
|
||||
|
||||
Args:
|
||||
options: The options to validate
|
||||
"""
|
||||
# Step 1: Plugin context validation
|
||||
self._validate_plugin_contexts(options)
|
||||
|
||||
# Step 2: Cross-cutting validation
|
||||
self._validate_cross_cutting_concerns(options)
|
||||
|
||||
def _validate_plugin_contexts(self, options: OCROptions) -> None:
|
||||
"""Validate plugin options that require external context."""
|
||||
if not self.registry:
|
||||
return
|
||||
|
||||
registered_models = self.registry.get_registered_models()
|
||||
|
||||
# Validate Tesseract options with language context
|
||||
if 'tesseract' in registered_models:
|
||||
from ocrmypdf.builtin_plugins.tesseract_ocr import TesseractOptions
|
||||
|
||||
# Create TesseractOptions from legacy fields for validation
|
||||
tesseract_data = {
|
||||
'config': options.tesseract_config,
|
||||
'pagesegmode': options.tesseract_pagesegmode,
|
||||
'oem': options.tesseract_oem,
|
||||
'thresholding': options.tesseract_thresholding,
|
||||
'timeout': options.tesseract_timeout,
|
||||
'non_ocr_timeout': options.tesseract_non_ocr_timeout or 180.0,
|
||||
'downsample_large_images': options.tesseract_downsample_large_images,
|
||||
'downsample_above': options.tesseract_downsample_above,
|
||||
'user_words': options.user_words,
|
||||
'user_patterns': options.user_patterns,
|
||||
}
|
||||
# Remove None values
|
||||
tesseract_data = {k: v for k, v in tesseract_data.items() if v is not None}
|
||||
|
||||
tesseract_options = TesseractOptions(**tesseract_data)
|
||||
tesseract_options.validate_with_context(options.languages)
|
||||
|
||||
# Validate Optimize options with external program context
|
||||
if 'optimize' in registered_models:
|
||||
from ocrmypdf.builtin_plugins.optimize import OptimizeOptions
|
||||
from ocrmypdf._exec import jbig2enc, pngquant
|
||||
|
||||
optimize_data = {
|
||||
'level': options.optimize,
|
||||
'jpeg_quality': options.jpeg_quality or 0,
|
||||
'png_quality': options.png_quality or 0,
|
||||
'jbig2_lossy': options.jbig2_lossy or False,
|
||||
'jbig2_page_group_size': options.jbig2_page_group_size or 0,
|
||||
'jbig2_threshold': options.jbig2_threshold,
|
||||
}
|
||||
|
||||
optimize_options = OptimizeOptions(**optimize_data)
|
||||
external_programs = {
|
||||
'pngquant': pngquant.available(),
|
||||
'jbig2enc': jbig2enc.available(),
|
||||
}
|
||||
optimize_options.validate_with_context(external_programs)
|
||||
|
||||
def _validate_cross_cutting_concerns(self, options: OCROptions) -> None:
|
||||
"""Validate cross-cutting concerns that span multiple plugins."""
|
||||
# Validate mutually exclusive OCR options
|
||||
exclusive_options = sum(
|
||||
1 for opt in [options.force_ocr, options.skip_text, options.redo_ocr] if opt
|
||||
)
|
||||
if exclusive_options >= 2:
|
||||
from ocrmypdf.exceptions import BadArgsError
|
||||
raise BadArgsError("Choose only one of --force-ocr, --skip-text, --redo-ocr.")
|
||||
|
||||
# Validate redo_ocr compatibility
|
||||
if options.redo_ocr:
|
||||
if options.deskew or options.clean_final or options.remove_background:
|
||||
from ocrmypdf.exceptions import BadArgsError
|
||||
raise BadArgsError(
|
||||
"--redo-ocr is not currently compatible with --deskew, "
|
||||
"--clean-final, and --remove-background"
|
||||
)
|
||||
|
||||
# Validate output type compatibility
|
||||
if options.output_type == 'none' and str(options.output_file) not in (
|
||||
os.devnull, '-'
|
||||
):
|
||||
from ocrmypdf.exceptions import BadArgsError
|
||||
raise BadArgsError(
|
||||
"Since you specified `--output-type none`, the output file "
|
||||
f"{options.output_file} cannot be produced. Set the output file to "
|
||||
"`-` to suppress this message."
|
||||
)
|
||||
|
||||
# Validate PDF/A image compression compatibility
|
||||
if (options.pdfa_image_compression and
|
||||
options.pdfa_image_compression != 'auto' and
|
||||
not options.output_type.startswith('pdfa')):
|
||||
log.warning(
|
||||
"--pdfa-image-compression argument only applies when "
|
||||
"--output-type is one of 'pdfa', 'pdfa-1', or 'pdfa-2'"
|
||||
)
|
||||
@@ -18,6 +18,7 @@ from ocrmypdf._pipeline import get_pdf_save_settings
|
||||
from ocrmypdf.cli import numeric
|
||||
from ocrmypdf.optimize import optimize
|
||||
from ocrmypdf.subprocess import check_external_program
|
||||
from pydantic import model_validator
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
@@ -139,6 +140,36 @@ class OptimizeOptions(BaseModel):
|
||||
),
|
||||
)
|
||||
|
||||
@model_validator(mode='after')
|
||||
def validate_optimization_consistency(self):
|
||||
"""Validate optimization options are consistent."""
|
||||
if self.level == 0 and any([
|
||||
self.jbig2_lossy,
|
||||
self.png_quality > 0,
|
||||
self.jpeg_quality > 0
|
||||
]):
|
||||
log.warning(
|
||||
"The arguments --jbig2-lossy, --png-quality, and --jpeg-quality "
|
||||
"will be ignored because --optimize=0."
|
||||
)
|
||||
return self
|
||||
|
||||
def validate_with_context(self, external_programs_available: dict[str, bool]) -> None:
|
||||
"""Validate options that require external context.
|
||||
|
||||
Args:
|
||||
external_programs_available: Dict of program name -> availability
|
||||
"""
|
||||
if self.level >= 2:
|
||||
if not external_programs_available.get('pngquant', False):
|
||||
log.warning(
|
||||
"pngquant is not available, so PNG optimization will be limited"
|
||||
)
|
||||
if not external_programs_available.get('jbig2enc', False):
|
||||
log.warning(
|
||||
"jbig2enc is not available, so JBIG2 optimization will be limited"
|
||||
)
|
||||
|
||||
|
||||
@hookimpl
|
||||
def register_options():
|
||||
@@ -154,6 +185,7 @@ def add_options(parser):
|
||||
|
||||
@hookimpl
|
||||
def check_options(options):
|
||||
"""Check external dependencies for optimization."""
|
||||
if options.optimize >= 2:
|
||||
check_external_program(
|
||||
program='pngquant',
|
||||
@@ -175,14 +207,6 @@ def check_options(options):
|
||||
recommended=True if not options.jbig2_lossy else False,
|
||||
)
|
||||
|
||||
if options.optimize == 0 and any(
|
||||
[options.jbig2_lossy, options.png_quality, options.jpeg_quality]
|
||||
):
|
||||
log.warning(
|
||||
"The arguments --jbig2-lossy, --png-quality, and --jpeg-quality "
|
||||
"will be ignored because --optimize=0."
|
||||
)
|
||||
|
||||
|
||||
@hookimpl
|
||||
def optimize_pdf(
|
||||
|
||||
@@ -21,6 +21,7 @@ from ocrmypdf.helpers import available_cpu_count, clamp
|
||||
from ocrmypdf.imageops import calculate_downsample, downsample_image
|
||||
from ocrmypdf.pluginspec import OcrEngine
|
||||
from ocrmypdf.subprocess import check_external_program
|
||||
from pydantic import field_validator, model_validator
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
@@ -204,6 +205,54 @@ class TesseractOptions(BaseModel):
|
||||
help="Specify the location of the Tesseract user patterns file.",
|
||||
)
|
||||
|
||||
@field_validator('timeout', 'non_ocr_timeout')
|
||||
@classmethod
|
||||
def validate_timeout_reasonable(cls, v):
|
||||
"""Validate timeout values are reasonable."""
|
||||
if v > 3600: # 1 hour
|
||||
log.warning(f"Timeout of {v} seconds is very long and may cause issues")
|
||||
return v
|
||||
|
||||
@field_validator('pagesegmode')
|
||||
@classmethod
|
||||
def validate_pagesegmode_warning(cls, v):
|
||||
"""Validate page segmentation mode and warn about problematic values."""
|
||||
if v in (0, 2):
|
||||
log.warning(
|
||||
"The tesseract-pagesegmode you selected will disable OCR. "
|
||||
"This may cause processing to fail."
|
||||
)
|
||||
return v
|
||||
|
||||
@model_validator(mode='after')
|
||||
def validate_downsample_consistency(self):
|
||||
"""Validate downsample options are consistent."""
|
||||
if (
|
||||
self.downsample_above != 32767
|
||||
and not self.downsample_large_images
|
||||
):
|
||||
log.warning(
|
||||
"The --tesseract-downsample-above argument will have no effect unless "
|
||||
"--tesseract-downsample-large-images is also given."
|
||||
)
|
||||
return self
|
||||
|
||||
def validate_with_context(self, languages: list[str]) -> None:
|
||||
"""Validate options that require external context.
|
||||
|
||||
Args:
|
||||
languages: List of languages being used for OCR
|
||||
"""
|
||||
# Validate languages are not internal Tesseract languages
|
||||
DENIED_LANGUAGES = {'equ', 'osd'}
|
||||
if DENIED_LANGUAGES & set(languages):
|
||||
raise BadArgsError(
|
||||
"The following languages are for Tesseract's internal use and should not "
|
||||
"be issued explicitly: "
|
||||
f"{', '.join(DENIED_LANGUAGES & set(languages))}\n"
|
||||
"Remove them from the -l/--language argument."
|
||||
)
|
||||
|
||||
|
||||
@hookimpl
|
||||
def register_options():
|
||||
@@ -219,6 +268,7 @@ def add_options(parser):
|
||||
|
||||
@hookimpl
|
||||
def check_options(options):
|
||||
"""Check external dependencies and version compatibility for Tesseract."""
|
||||
check_external_program(
|
||||
program='tesseract',
|
||||
package={'linux': 'tesseract-ocr'},
|
||||
@@ -233,27 +283,13 @@ def check_options(options):
|
||||
"Please upgrade to a newer or supported older version."
|
||||
)
|
||||
|
||||
# Validate Tesseract-specific options using the new model
|
||||
# For now, we still access options directly for backward compatibility
|
||||
# Check version-specific feature compatibility
|
||||
if not tesseract.has_thresholding() and options.tesseract_thresholding != 0:
|
||||
log.warning(
|
||||
"The installed version of Tesseract does not support changes to its "
|
||||
"thresholding method. The --tesseract-threshold argument will be "
|
||||
"ignored."
|
||||
)
|
||||
if options.tesseract_pagesegmode in (0, 2):
|
||||
log.warning(
|
||||
"The --tesseract-pagesegmode argument you select will disable OCR. "
|
||||
"This may cause processing to fail."
|
||||
)
|
||||
DENIED_LANGUAGES = {'equ', 'osd'}
|
||||
if DENIED_LANGUAGES & set(options.languages):
|
||||
raise BadArgsError(
|
||||
"The following languages for Tesseract's internal use and should not "
|
||||
"be issued explicitly: "
|
||||
f"{', '.join(DENIED_LANGUAGES & set(options.languages))}\n"
|
||||
"Remove them from the -l/--language argument."
|
||||
)
|
||||
|
||||
|
||||
@hookimpl
|
||||
|
||||
Reference in New Issue
Block a user