fix(CODEWIKI-002): CU-86akhf8u6 8 review findings across 3 files

codewiki/cli/models/job.py

@@ -1,5 +1,32 @@
 """
 Documentation job data models.
+
+This module defines the persisted data structures used to track the
+lifecycle of a documentation generation job in the CLI pipeline. A
+DocumentationJob represents a single run of the documentation generator
+against a repository: it records where the repository lives, which commit
+and branch were processed, the status of the run, any error encountered,
+the files produced, and nested configuration/statistics objects describing
+how the run was performed and what it produced.
+
+Responsibilities:
+    - JobStatus: enumerates the possible lifecycle states of a job
+      (pending, running, completed, failed).
+    - GenerationOptions: captures user-selected options that influence how
+      documentation is generated (branch creation, GitHub Pages
+      publishing, cache usage, custom output location).
+    - LLMConfig: captures which LLM models and endpoint were used to
+      generate documentation for a given job.
+    - JobStatistics: captures metrics collected while generating
+      documentation (files analyzed, leaf node count, depth, tokens used).
+    - DocumentationJob: the aggregate root that is persisted to disk as
+      JSON so that job state can survive process restarts and be inspected
+      by CLI commands (e.g. status, resume, history).
+
+Each persisted dataclass owns its own to_dict()/from_dict() methods so
+that the field list for serialization lives in exactly one place per
+class, avoiding drift between the in-memory representation and the JSON
+representation used for storage.
 """
 
 from dataclasses import dataclass, field
@@ -18,6 +45,16 @@ class JobStatus(str, Enum):
     FAILED = "failed"
 
 
+def _coerce_int(value: Any, default: int = 0) -> int:
+    """Coerce a raw value into an int, falling back to a default."""
+    if value is None:
+        return default
+    try:
+        return int(value)
+    except (TypeError, ValueError):
+        return default
+
+
 @dataclass
 class GenerationOptions:
     """Options for documentation generation."""
@@ -26,6 +63,29 @@ class GenerationOptions:
     no_cache: bool = False
     custom_output: Optional[str] = None
 
+    def to_dict(self) -> Dict[str, Any]:
+        """Convert to dictionary for JSON serialization."""
+        return {
+            "create_branch": self.create_branch,
+            "github_pages": self.github_pages,
+            "no_cache": self.no_cache,
+            "custom_output": self.custom_output,
+        }
+
+    @classmethod
+    def from_dict(cls, data: Any) -> 'GenerationOptions':
+        """Create from dictionary."""
+        if isinstance(data, GenerationOptions):
+            return data
+        if not data:
+            return cls()
+        return cls(
+            create_branch=bool(data.get('create_branch', False)),
+            github_pages=bool(data.get('github_pages', False)),
+            no_cache=bool(data.get('no_cache', False)),
+            custom_output=data.get('custom_output'),
+        )
+
 
 @dataclass
 class JobStatistics:
@@ -35,6 +95,29 @@ class JobStatistics:
     max_depth: int = 0
     total_tokens_used: int = 0
 
+    def to_dict(self) -> Dict[str, Any]:
+        """Convert to dictionary for JSON serialization."""
+        return {
+            "total_files_analyzed": self.total_files_analyzed,
+            "leaf_nodes": self.leaf_nodes,
+            "max_depth": self.max_depth,
+            "total_tokens_used": self.total_tokens_used,
+        }
+
+    @classmethod
+    def from_dict(cls, data: Any) -> 'JobStatistics':
+        """Create from dictionary."""
+        if isinstance(data, JobStatistics):
+            return data
+        if not data:
+            return cls()
+        return cls(
+            total_files_analyzed=_coerce_int(data.get('total_files_analyzed'), 0),
+            leaf_nodes=_coerce_int(data.get('leaf_nodes'), 0),
+            max_depth=_coerce_int(data.get('max_depth'), 0),
+            total_tokens_used=_coerce_int(data.get('total_tokens_used'), 0),
+        )
+
 
 @dataclass
 class LLMConfig:
@@ -43,6 +126,27 @@ class LLMConfig:
     cluster_model: str
     base_url: str
 
+    def to_dict(self) -> Dict[str, Any]:
+        """Convert to dictionary for JSON serialization."""
+        return {
+            "main_model": self.main_model,
+            "cluster_model": self.cluster_model,
+            "base_url": self.base_url,
+        }
+
+    @classmethod
+    def from_dict(cls, data: Any) -> Optional['LLMConfig']:
+        """Create from dictionary, or None if data is empty."""
+        if isinstance(data, LLMConfig):
+            return data
+        if not data:
+            return None
+        return cls(
+            main_model=data.get('main_model', ''),
+            cluster_model=data.get('cluster_model', ''),
+            base_url=data.get('base_url', ''),
+        )
+
 
 def _coerce_job_status(value: Any, default: JobStatus = JobStatus.PENDING) -> JobStatus:
     """Coerce a raw value into a JobStatus, falling back to a default."""
@@ -56,57 +160,6 @@ def _coerce_job_status(value: Any, default: JobStatus = JobStatus.PENDING) -> Jo
         return default
 
 
-def _coerce_int(value: Any, default: int = 0) -> int:
-    """Coerce a raw value into an int, falling back to a default."""
-    if value is None:
-        return default
-    try:
-        return int(value)
-    except (TypeError, ValueError):
-        return default
-
-
-def _coerce_generation_options(value: Any) -> GenerationOptions:
-    """Coerce a raw dict into a GenerationOptions instance."""
-    if isinstance(value, GenerationOptions):
-        return value
-    if not value:
-        return GenerationOptions()
-    return GenerationOptions(
-        create_branch=bool(value.get('create_branch', False)),
-        github_pages=bool(value.get('github_pages', False)),
-        no_cache=bool(value.get('no_cache', False)),
-        custom_output=value.get('custom_output'),
-    )
-
-
-def _coerce_llm_config(value: Any) -> Optional[LLMConfig]:
-    """Coerce a raw dict into an LLMConfig instance, or None."""
-    if isinstance(value, LLMConfig):
-        return value
-    if not value:
-        return None
-    return LLMConfig(
-        main_model=value.get('main_model', ''),
-        cluster_model=value.get('cluster_model', ''),
-        base_url=value.get('base_url', ''),
-    )
-
-
-def _coerce_statistics(value: Any) -> JobStatistics:
-    """Coerce a raw dict into a JobStatistics instance."""
-    if isinstance(value, JobStatistics):
-        return value
-    if not value:
-        return JobStatistics()
-    return JobStatistics(
-        total_files_analyzed=_coerce_int(value.get('total_files_analyzed'), 0),
-        leaf_nodes=_coerce_int(value.get('leaf_nodes'), 0),
-        max_depth=_coerce_int(value.get('max_depth'), 0),
-        total_tokens_used=_coerce_int(value.get('total_tokens_used'), 0),
-    )
-
-
 @dataclass
 class DocumentationJob:
     """
@@ -176,23 +229,9 @@ def to_dict(self) -> Dict[str, Any]:
             "error_message": self.error_message,
             "files_generated": self.files_generated,
             "module_count": self.module_count,
-            "generation_options": {
-                "create_branch": self.generation_options.create_branch,
-                "github_pages": self.generation_options.github_pages,
-                "no_cache": self.generation_options.no_cache,
-                "custom_output": self.generation_options.custom_output,
-            },
-            "llm_config": {
-                "main_model": self.llm_config.main_model,
-                "cluster_model": self.llm_config.cluster_model,
-                "base_url": self.llm_config.base_url,
-            } if self.llm_config else None,
-            "statistics": {
-                "total_files_analyzed": self.statistics.total_files_analyzed,
-                "leaf_nodes": self.statistics.leaf_nodes,
-                "max_depth": self.statistics.max_depth,
-                "total_tokens_used": self.statistics.total_tokens_used,
-            },
+            "generation_options": self.generation_options.to_dict(),
+            "llm_config": self.llm_config.to_dict() if self.llm_config else None,
+            "statistics": self.statistics.to_dict(),
         }
         return data
 
@@ -220,13 +259,14 @@ def from_dict(cls, data: Dict[str, Any]) -> 'DocumentationJob':
 
         # Parse nested objects
         if 'generation_options' in data:
-            job.generation_options = _coerce_generation_options(data['generation_options'])
+            job.generation_options = GenerationOptions.from_dict(data['generation_options'])
 
         if 'llm_config' in data and data['llm_config']:
-            job.llm_config = _coerce_llm_config(data['llm_config'])
+            job.llm_config = LLMConfig.from_dict(data['llm_config'])
 
         if 'statistics' in data:
-            job.statistics = _coerce_statistics(data['statistics'])
+            job.statistics = JobStatistics.from_dict(data['statistics'])
 
         return job
 
+

codewiki/src/fe/models.py

@@ -1,11 +1,50 @@
 #!/usr/bin/env python3
 """
 Data models and classes for the CodeWiki web application.
+
+This module defines the data types that flow through the CodeWiki
+documentation-generation pipeline:
+
+- RepositorySubmission: the pydantic request model accepted by the API
+  when a user submits a repository URL to be documented.
+- JobStatusResponse: the pydantic response model returned by the API
+  when reporting the status of a documentation generation job.
+- JobStatus: the dataclass used internally by the background worker to
+  track the lifecycle of a documentation generation job (queued ->
+  processing -> completed/failed), and which is serialized at the
+  boundary between the background worker, the API layer, and the cache.
+- CacheEntry: the dataclass representing a cached documentation result,
+  persisted and read back by the cache manager.
+
+JobStatus and CacheEntry provide explicit to_dict()/from_dict()
+serialization helpers so that all boundary crossings (background worker
+-> API response -> cache) go through a single, type-safe conversion
+point rather than ad-hoc dict construction elsewhere in the codebase.
 """
 
 from datetime import datetime
-from typing import Optional
-from dataclasses import dataclass
+from typing import Optional, Any, Dict
+from dataclasses import dataclass, asdict
+
+
+def _coerce_datetime(value: Any) -> Optional[datetime]:
+    """Coerce a value into a datetime instance, or None."""
+    if value is None:
+        return None
+    if isinstance(value, datetime):
+        return value
+    if isinstance(value, str):
+        return datetime.fromisoformat(value)
+    raise TypeError(f"Cannot coerce value of type {type(value)!r} to datetime")
+
+
+def _datetime_to_iso(value: Optional[datetime]) -> Optional[str]:
+    """Convert a datetime (or None) into its ISO 8601 string representation."""
+    if value is None:
+        return None
+    return value.isoformat()
+
+
 from pydantic import BaseModel, HttpUrl
 
 
@@ -44,6 +83,31 @@ class JobStatus:
     main_model: Optional[str] = None
     commit_id: Optional[str] = None
 
+    def to_dict(self) -> Dict[str, Any]:
+        """Serialize this JobStatus to a plain dict with ISO-formatted datetimes."""
+        data = asdict(self)
+        data["created_at"] = _datetime_to_iso(self.created_at)
+        data["started_at"] = _datetime_to_iso(self.started_at)
+        data["completed_at"] = _datetime_to_iso(self.completed_at)
+        return data
+
+    @classmethod
+    def from_dict(cls, data: Dict[str, Any]) -> "JobStatus":
+        """Deserialize a JobStatus from a plain dict, coercing datetime fields."""
+        return cls(
+            job_id=data["job_id"],
+            repo_url=data["repo_url"],
+            status=data["status"],
+            created_at=_coerce_datetime(data["created_at"]),
+            started_at=_coerce_datetime(data.get("started_at")),
+            completed_at=_coerce_datetime(data.get("completed_at")),
+            error_message=data.get("error_message"),
+            progress=data.get("progress", ""),
+            docs_path=data.get("docs_path"),
+            main_model=data.get("main_model"),
+            commit_id=data.get("commit_id"),
+        )
+
 
 @dataclass
 class CacheEntry:
@@ -52,4 +116,25 @@ class CacheEntry:
     repo_url_hash: str
     docs_path: str
     created_at: datetime
-    last_accessed: datetime
+    last_accessed: datetime
+
+    def to_dict(self) -> Dict[str, Any]:
+        """Serialize this CacheEntry to a plain dict with ISO-formatted datetimes."""
+        return {
+            "repo_url": self.repo_url,
+            "repo_url_hash": self.repo_url_hash,
+            "docs_path": self.docs_path,
+            "created_at": _datetime_to_iso(self.created_at),
+            "last_accessed": _datetime_to_iso(self.last_accessed),
+        }
+
+    @classmethod
+    def from_dict(cls, data: Dict[str, Any]) -> "CacheEntry":
+        """Deserialize a CacheEntry from a plain dict, coercing datetime fields."""
+        return cls(
+            repo_url=data["repo_url"],
+            repo_url_hash=data["repo_url_hash"],
+            docs_path=data["docs_path"],
+            created_at=_coerce_datetime(data["created_at"]),
+            last_accessed=_coerce_datetime(data["last_accessed"]),
+        )