fix(CODEWIKI-002): CU-86akn96pk 2 review findings in models.py - #93
flamingo[bot] wants to merge 1 commit into
Conversation
![flamingo[bot]](https://avatars.githubusercontent.com/in/3426709?s=60&v=4){kind=small}
| main_model: Optional[str] = None | ||
| commit_id: Optional[str] = None | ||
|
|
||
| def to_dict(self) -> Dict[str, Any]: | ||
| """Serialize this JobStatus to a plain dict.""" | ||
| return { | ||
| "job_id": self.job_id, | ||
| "repo_url": self.repo_url, | ||
| "status": self.status, | ||
| "created_at": self.created_at.isoformat() if self.created_at else None, | ||
| "started_at": self.started_at.isoformat() if self.started_at else None, | ||
| "completed_at": self.completed_at.isoformat() if self.completed_at else None, | ||
| "error_message": self.error_message, | ||
| "progress": self.progress, | ||
| "docs_path": self.docs_path, | ||
| "main_model": self.main_model, | ||
| "commit_id": self.commit_id, | ||
| } | ||
|
|
||
| @classmethod | ||
| def from_dict(cls, data: Dict[str, Any]) -> "JobStatus": | ||
| """Deserialize a JobStatus from a plain dict.""" | ||
| created_at = data.get("created_at") | ||
| started_at = data.get("started_at") | ||
| completed_at = data.get("completed_at") | ||
| return cls( | ||
| job_id=data["job_id"], | ||
| repo_url=data["repo_url"], | ||
| status=data["status"], | ||
| created_at=datetime.fromisoformat(created_at) if isinstance(created_at, str) else created_at, | ||
| started_at=datetime.fromisoformat(started_at) if isinstance(started_at, str) else started_at, | ||
| completed_at=datetime.fromisoformat(completed_at) if isinstance(completed_at, str) else 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: |
There was a problem hiding this comment.
🦩 🟠 JobStatus, CacheEntry dataclasses lack to_dict/from_dict methods
Added to_dict/from_dict methods to the JobStatus dataclass (codewiki/src/fe/models.py, lines within the JobStatus class). to_dict explicitly maps all fields to a plain dict (converting datetimes to ISO strings), and from_dict reconstructs a JobStatus from a dict (parsing ISO datetime strings back). This is a self-contained addition to models.py; cache_manager.py was not modified (out of scope for this file-only fix), so the reviewer should follow up there to replace the hand-rolled logic with calls to these new methods.
🤖 Prompt for AI agents
In codewiki/src/fe/models.py around line 32, review and complete this code-review fix: JobStatus, CacheEntry dataclasses lack to_dict/from_dict methods.
What the draft fix changed: Added `to_dict`/`from_dict` methods to the `JobStatus` dataclass (codewiki/src/fe/models.py, lines within the `JobStatus` class). `to_dict` explicitly maps all fields to a plain dict (converting datetimes to ISO strings), and `from_dict` reconstructs a `JobStatus` from a dict (parsing ISO datetime strings back). This is a self-contained addition to models.py; cache_manager.py was not modified (out of scope for this file-only fix), so the reviewer should follow up there to replace the hand-rolled logic with calls to these new methods.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟡 70 medium — react 👍/👎 to teach the reviewer
| ) | ||
|
|
||
|
|
||
| @dataclass |
There was a problem hiding this comment.
🦩 🟠 CacheEntry dataclass lacks to_dict/from_dict; serialization duplicated in CacheManager
Added to_dict/from_dict methods to the CacheEntry dataclass (codewiki/src/fe/models.py, lines within the CacheEntry class), explicitly handling the five named fields (repo_url, repo_url_hash, docs_path, created_at, last_accessed) including datetime ISO conversion. As with JobStatus, cache_manager.py's load_cache_index/save_cache_index were not touched since only models.py was in scope; a complete fix requires updating cache_manager.py to call CacheEntry.to_dict()/CacheEntry.from_dict() instead of manual dict construction.
🤖 Prompt for AI agents
In codewiki/src/fe/models.py around line 48, review and complete this code-review fix: CacheEntry dataclass lacks to_dict/from_dict; serialization duplicated in CacheManager.
What the draft fix changed: Added `to_dict`/`from_dict` methods to the `CacheEntry` dataclass (codewiki/src/fe/models.py, lines within the `CacheEntry` class), explicitly handling the five named fields (`repo_url`, `repo_url_hash`, `docs_path`, `created_at`, `last_accessed`) including datetime ISO conversion. As with JobStatus, cache_manager.py's load_cache_index/save_cache_index were not touched since only models.py was in scope; a complete fix requires updating cache_manager.py to call `CacheEntry.to_dict()`/`CacheEntry.from_dict()` instead of manual dict construction.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟡 70 medium — react 👍/👎 to teach the reviewer
flamingo
Bot
changed the title
Closes 2 review findings in
codewiki/src/fe/models.py.Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.
codewiki/src/fe/models.py:32codewiki/src/fe/models.py:48What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.
Run: https://product-hub.flamingo.so/admin/code-review
Run id:
90e36f49-f8c5-4b9a-a9b0-2d911418acb4Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.
ClickUp task: CU-86akn96pk CodeWiki review findings sweep (9 PRs)