diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index cd7d5ff..d4de6b0 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -5,8 +5,3 @@ repos: - id: ruff args: [--fix] - id: ruff-format - - - repo: https://github.com/psf/black - rev: 25.1.0 - hooks: - - id: black diff --git a/mistral_ocr/cli.py b/mistral_ocr/cli.py index 013142c..1cb172a 100644 --- a/mistral_ocr/cli.py +++ b/mistral_ocr/cli.py @@ -6,16 +6,13 @@ from pathlib import Path import click -from rich.console import Console from rich.logging import RichHandler from . import __version__ from .config import Config -from .processor import OCRProcessor +from .processor import OCRProcessor, console from .utils import format_file_size, get_supported_files -console = Console() - # Get the original working directory if set ORIGINAL_CWD = os.environ.get("MISTRAL_OCR_CWD", os.getcwd()) @@ -146,11 +143,11 @@ def main( if output_path and not output_path.is_absolute(): output_path = Path(ORIGINAL_CWD) / output_path - # Quiet mode: suppress non-error output + # Quiet mode: suppress non-error output (uses processor's shared console) if quiet: console.quiet = True - # Configure logging + # Configure logging — use verbose flag now, may upgrade later from config log_level = logging.DEBUG if verbose else logging.WARNING handlers: list[logging.Handler] = [] if not quiet: @@ -168,7 +165,24 @@ def main( console.print("\n[bold blue]🔍 Mistral OCR[/bold blue]") console.print("[dim]Powered by Mistral AI's OCR API[/dim]\n") - # Load configuration + # Dry-run: list files that would be processed, then exit (no API key needed) + if dry_run: + if input_path.is_file(): + size = format_file_size(input_path.stat().st_size) + console.print(f" {input_path.name} ({size})") + console.print("\n[dim]1 file would be processed (dry run)[/dim]") + elif input_path.is_dir(): + files = get_supported_files(input_path) + if not files: + console.print("[yellow]No supported files found.[/yellow]") + else: + for f in files: + size = format_file_size(f.stat().st_size) + console.print(f" {f.relative_to(input_path)} ({size})") + console.print(f"\n[dim]{len(files)} file(s) would be processed (dry run)[/dim]") + return + + # Load configuration (requires API key — after dry-run check) # If API key is provided via CLI, set it before loading config # (must happen before load_dotenv, which won't override existing vars) @@ -178,6 +192,10 @@ def main( # Create config from environment config = Config.from_env(env_file) + # If config has VERBOSE=true but CLI didn't pass --verbose, upgrade log level + if config.verbose and not verbose: + logging.getLogger().setLevel(logging.DEBUG) + # Only override config with CLI options that were explicitly passed ctx = click.get_current_context() if ( @@ -216,26 +234,8 @@ def main( ): config.extract_footer = extract_footers - config.dry_run = dry_run config.quiet = quiet - # Dry-run: list files that would be processed, then exit - if dry_run: - if input_path.is_file(): - size = format_file_size(input_path.stat().st_size) - console.print(f" {input_path.name} ({size})") - console.print("\n[dim]1 file would be processed (dry run)[/dim]") - elif input_path.is_dir(): - files = get_supported_files(input_path) - if not files: - console.print("[yellow]No supported files found.[/yellow]") - else: - for f in files: - size = format_file_size(f.stat().st_size) - console.print(f" {f.relative_to(input_path)} ({size})") - console.print(f"\n[dim]{len(files)} file(s) would be processed (dry run)[/dim]") - return - # Create processor processor = OCRProcessor(config) diff --git a/mistral_ocr/config.py b/mistral_ocr/config.py index 92f97d8..1b824d8 100644 --- a/mistral_ocr/config.py +++ b/mistral_ocr/config.py @@ -50,6 +50,8 @@ def from_env(cls, env_file: Path | None = None) -> "Config": raise ValueError( f"MAX_FILE_SIZE_MB must be an integer, got: {os.getenv('MAX_FILE_SIZE_MB')!r}" ) from e + if max_file_size_mb <= 0: + raise ValueError(f"MAX_FILE_SIZE_MB must be positive, got: {max_file_size_mb}") try: max_retries = int(os.getenv("MAX_RETRIES", "3")) @@ -57,6 +59,8 @@ def from_env(cls, env_file: Path | None = None) -> "Config": raise ValueError( f"MAX_RETRIES must be an integer, got: {os.getenv('MAX_RETRIES')!r}" ) from e + if max_retries < 0: + raise ValueError(f"MAX_RETRIES must be non-negative, got: {max_retries}") try: retry_base_delay = float(os.getenv("RETRY_BASE_DELAY", "1.0")) @@ -64,6 +68,8 @@ def from_env(cls, env_file: Path | None = None) -> "Config": raise ValueError( f"RETRY_BASE_DELAY must be a number, got: {os.getenv('RETRY_BASE_DELAY')!r}" ) from e + if retry_base_delay < 0: + raise ValueError(f"RETRY_BASE_DELAY must be non-negative, got: {retry_base_delay}") return cls( api_key=api_key, diff --git a/mistral_ocr/processor.py b/mistral_ocr/processor.py index b97c6a4..08ac5ba 100644 --- a/mistral_ocr/processor.py +++ b/mistral_ocr/processor.py @@ -24,6 +24,8 @@ ) logger = logging.getLogger(__name__) + +# Shared console instance — CLI sets .quiet on this directly console = Console() @@ -401,22 +403,31 @@ def process( result = self.process_file(input_path) if result: - self.save_results(result, output_dir, is_single_file=True) - base_name = make_unique_basename(input_path) - self.processed_files.append( - { - "file": str(input_path.resolve()), - "size": input_path.stat().st_size, - "output": str(output_dir / base_name / f"{base_name}.md"), - } - ) + try: + self.save_results(result, output_dir, is_single_file=True) + base_name = make_unique_basename(input_path) + self.processed_files.append( + { + "file": str(input_path.resolve()), + "size": input_path.stat().st_size, + "output": str(output_dir / base_name / f"{base_name}.md"), + } + ) + except (OSError, ValueError) as e: + console.print(f"[red]Error saving results for {input_path.name}: {e}[/red]") + self.errors.append( + {"file": str(input_path.resolve()), "error": f"Save failed: {e}"} + ) # Save metadata processing_time = time.time() - start_time save_metadata(output_dir, self.processed_files, processing_time, self.errors) - console.print("\n[green]✓ Successfully processed 1 file[/green]") - console.print(f"[dim]Processing time: {processing_time:.2f} seconds[/dim]") + if self.errors: + console.print("\n[red]✗ Failed to save results[/red]") + else: + console.print("\n[green]✓ Successfully processed 1 file[/green]") + console.print(f"[dim]Processing time: {processing_time:.2f} seconds[/dim]") else: console.print("\n[red]✗ Failed to process file[/red]") diff --git a/tests/test_cli.py b/tests/test_cli.py index d9cbe21..86c69d1 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -57,6 +57,26 @@ def test_dry_run_does_not_call_api(self, runner, tmp_path, monkeypatch): assert result.exit_code == 0 mock.assert_not_called() + def test_dry_run_works_without_api_key(self, runner, tmp_path, monkeypatch): + """Dry-run must not require MISTRAL_API_KEY.""" + monkeypatch.delenv("MISTRAL_API_KEY", raising=False) + pdf = tmp_path / "doc.pdf" + pdf.write_bytes(b"%PDF-1.4 test") + + result = runner.invoke(main, [str(pdf), "--dry-run"]) + assert result.exit_code == 0 + assert "doc.pdf" in result.output + assert "dry run" in result.output + + def test_dry_run_directory_without_api_key(self, runner, tmp_path, monkeypatch): + """Dry-run on a directory must not require MISTRAL_API_KEY.""" + monkeypatch.delenv("MISTRAL_API_KEY", raising=False) + (tmp_path / "a.pdf").write_bytes(b"%PDF") + + result = runner.invoke(main, [str(tmp_path), "--dry-run"]) + assert result.exit_code == 0 + assert "a.pdf" in result.output + class TestQuiet: def test_quiet_suppresses_output(self, runner, tmp_path, monkeypatch): @@ -74,3 +94,24 @@ def test_quiet_suppresses_output(self, runner, tmp_path, monkeypatch): # Quiet mode: no banner, no completion message assert "Mistral OCR" not in result.output assert "Processing complete" not in result.output + + def test_quiet_propagates_to_processor_console(self, runner, tmp_path, monkeypatch): + """Quiet flag must set .quiet on the shared processor console.""" + _make_env(monkeypatch) + pdf = tmp_path / "doc.pdf" + pdf.write_bytes(b"%PDF-1.4 test") + + with patch("mistral_ocr.cli.OCRProcessor") as mock_cls: + mock_proc = mock_cls.return_value + mock_proc.errors = [] + mock_proc.process.return_value = None + + runner.invoke(main, [str(pdf), "--quiet"]) + + # The console imported from processor should be quiet + from mistral_ocr.processor import console as proc_console + + assert proc_console.quiet is True + + # Reset for other tests + proc_console.quiet = False diff --git a/tests/test_config.py b/tests/test_config.py index 1510836..fdb63cd 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -95,6 +95,23 @@ def test_config_from_env_invalid_numeric(monkeypatch, env_var, bad_value, expect Config.from_env() +@pytest.mark.parametrize( + "env_var,bad_value,expected_msg", + [ + ("MAX_FILE_SIZE_MB", "0", "MAX_FILE_SIZE_MB must be positive"), + ("MAX_FILE_SIZE_MB", "-5", "MAX_FILE_SIZE_MB must be positive"), + ("MAX_RETRIES", "-1", "MAX_RETRIES must be non-negative"), + ("RETRY_BASE_DELAY", "-0.5", "RETRY_BASE_DELAY must be non-negative"), + ], +) +def test_config_from_env_negative_values(monkeypatch, env_var, bad_value, expected_msg): + """Test Config.from_env rejects negative/zero numeric env vars.""" + monkeypatch.setenv("MISTRAL_API_KEY", "key") + monkeypatch.setenv(env_var, bad_value) + with pytest.raises(ValueError, match=expected_msg): + Config.from_env() + + def test_validate_file_size(tmp_path): """Test file size validation.""" config = Config(api_key="key", max_file_size_mb=1)