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
This commit is contained in:
parent
19fa3e31a8
commit
f9a1c56309
3 changed files with 83 additions and 7 deletions
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
Loading…
Reference in a new issue