From f9a1c563094049384594ae876e1272e42a3c8faa Mon Sep 17 00:00:00 2001 From: Pier-Jean Malandrino Date: Fri, 10 Apr 2026 12:04:44 +0200 Subject: [PATCH] fix(domain): add guard clauses and frozen value objects Add state-machine guard clauses to AnalysisJob transition methods (mark_running, mark_completed, mark_failed, update_progress) to prevent invalid status transitions. Make all domain value objects immutable with frozen=True. Add 11 tests covering guard clause behavior. Closes #132, closes #133 --- document-parser/domain/models.py | 10 ++++ document-parser/domain/value_objects.py | 14 +++--- document-parser/tests/test_models.py | 66 +++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 7 deletions(-) diff --git a/document-parser/domain/models.py b/document-parser/domain/models.py index 036bc88..e89883d 100644 --- a/document-parser/domain/models.py +++ b/document-parser/domain/models.py @@ -56,6 +56,8 @@ class AnalysisJob: def mark_running(self) -> None: """Transition to RUNNING and record the start timestamp.""" + if self.status != AnalysisStatus.PENDING: + raise ValueError(f"Cannot mark as RUNNING from {self.status} (expected PENDING)") self.status = AnalysisStatus.RUNNING self.started_at = _utcnow() @@ -68,6 +70,8 @@ class AnalysisJob: chunks_json: str | None = None, ) -> None: """Transition to COMPLETED with conversion results.""" + if self.status != AnalysisStatus.RUNNING: + raise ValueError(f"Cannot mark as COMPLETED from {self.status} (expected RUNNING)") self.status = AnalysisStatus.COMPLETED self.content_markdown = markdown self.content_html = html @@ -78,11 +82,17 @@ class AnalysisJob: def update_progress(self, current: int, total: int) -> None: """Update batch progress counters.""" + if self.status != AnalysisStatus.RUNNING: + raise ValueError(f"Cannot update progress from {self.status} (expected RUNNING)") self.progress_current = current self.progress_total = total def mark_failed(self, error: str) -> None: """Transition to FAILED with an error message.""" + if self.status not in (AnalysisStatus.PENDING, AnalysisStatus.RUNNING): + raise ValueError( + f"Cannot mark as FAILED from {self.status} (expected PENDING or RUNNING)" + ) self.status = AnalysisStatus.FAILED self.error_message = error self.completed_at = _utcnow() diff --git a/document-parser/domain/value_objects.py b/document-parser/domain/value_objects.py index 7c91845..f633117 100644 --- a/document-parser/domain/value_objects.py +++ b/document-parser/domain/value_objects.py @@ -9,7 +9,7 @@ from __future__ import annotations from dataclasses import dataclass, field -@dataclass +@dataclass(frozen=True) class PageElement: type: str bbox: list[float] @@ -17,7 +17,7 @@ class PageElement: level: int = 0 -@dataclass +@dataclass(frozen=True) class PageDetail: page_number: int width: float @@ -25,7 +25,7 @@ class PageDetail: elements: list[PageElement] = field(default_factory=list) -@dataclass +@dataclass(frozen=True) class ConversionOptions: do_ocr: bool = True do_table_structure: bool = True @@ -43,7 +43,7 @@ class ConversionOptions: return self == ConversionOptions() -@dataclass +@dataclass(frozen=True) class ConversionResult: page_count: int content_markdown: str @@ -53,7 +53,7 @@ class ConversionResult: document_json: str | None = None -@dataclass +@dataclass(frozen=True) class ChunkingOptions: chunker_type: str = "hybrid" # "hybrid", "hierarchical", "page" max_tokens: int = 512 @@ -65,13 +65,13 @@ class ChunkingOptions: return self == ChunkingOptions() -@dataclass +@dataclass(frozen=True) class ChunkBbox: page: int bbox: list[float] # [left, top, right, bottom] in TOPLEFT origin -@dataclass +@dataclass(frozen=True) class ChunkResult: text: str headings: list[str] = field(default_factory=list) diff --git a/document-parser/tests/test_models.py b/document-parser/tests/test_models.py index 0098e6a..8d24540 100644 --- a/document-parser/tests/test_models.py +++ b/document-parser/tests/test_models.py @@ -2,6 +2,8 @@ from datetime import datetime +import pytest + from domain.models import AnalysisJob, AnalysisStatus, Document @@ -96,6 +98,70 @@ class TestAnalysisJob: assert job.status == AnalysisStatus.COMPLETED +class TestAnalysisJobGuardClauses: + """Guard clauses prevent invalid state transitions.""" + + def test_mark_running_from_running_raises(self): + job = AnalysisJob() + job.mark_running() + with pytest.raises(ValueError, match="Cannot mark as RUNNING"): + job.mark_running() + + def test_mark_running_from_completed_raises(self): + job = AnalysisJob() + job.mark_running() + job.mark_completed(markdown="", html="", pages_json="[]") + with pytest.raises(ValueError, match="Cannot mark as RUNNING"): + job.mark_running() + + def test_mark_running_from_failed_raises(self): + job = AnalysisJob() + job.mark_failed("err") + with pytest.raises(ValueError, match="Cannot mark as RUNNING"): + job.mark_running() + + def test_mark_completed_from_pending_raises(self): + job = AnalysisJob() + with pytest.raises(ValueError, match="Cannot mark as COMPLETED"): + job.mark_completed(markdown="", html="", pages_json="[]") + + def test_mark_completed_from_failed_raises(self): + job = AnalysisJob() + job.mark_failed("err") + with pytest.raises(ValueError, match="Cannot mark as COMPLETED"): + job.mark_completed(markdown="", html="", pages_json="[]") + + def test_mark_failed_from_completed_raises(self): + job = AnalysisJob() + job.mark_running() + job.mark_completed(markdown="", html="", pages_json="[]") + with pytest.raises(ValueError, match="Cannot mark as FAILED"): + job.mark_failed("err") + + def test_mark_failed_from_pending_allowed(self): + job = AnalysisJob() + job.mark_failed("err") + assert job.status == AnalysisStatus.FAILED + + def test_mark_failed_from_running_allowed(self): + job = AnalysisJob() + job.mark_running() + job.mark_failed("err") + assert job.status == AnalysisStatus.FAILED + + def test_update_progress_from_pending_raises(self): + job = AnalysisJob() + with pytest.raises(ValueError, match="Cannot update progress"): + job.update_progress(1, 10) + + def test_update_progress_from_running_allowed(self): + job = AnalysisJob() + job.mark_running() + job.update_progress(5, 10) + assert job.progress_current == 5 + assert job.progress_total == 10 + + class TestAnalysisStatus: def test_values(self): assert AnalysisStatus.PENDING == "PENDING"