Fix quiet mode, dry-run, and config validation bugs

.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

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)
 

mistral_ocr/config.py

@@ -50,20 +50,26 @@ 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"))
         except ValueError as e:
             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"))
         except ValueError as e:
             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,

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]")
 

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

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)