fix(CODEWIKI-002): 2 review findings in job.py - #38
Conversation
![flamingo[bot]](https://avatars.githubusercontent.com/in/3426709?s=60&v=4){kind=small}
| @@ -100,6 +134,30 @@ def fail(self, error_message: str): | |||
|
|
|||
There was a problem hiding this comment.
🦩 🔴 DocumentationJob.to_dict delegates to asdict() for nested dataclasses instead of explicit field listing
In DocumentationJob.to_dict, replaced asdict(self.generation_options), asdict(self.llm_config), and asdict(self.statistics) with explicit dict literals listing each field by name. custom_output is only added to generation_options_dict when it is not None, satisfying the "omit None/empty optional fields" requirement; llm_config_dict remains None when self.llm_config is falsy.
🤖 Prompt for AI agents
In codewiki/cli/models/job.py around line 100, review and complete this code-review fix: DocumentationJob.to_dict delegates to asdict() for nested dataclasses instead of explicit field listing.
What the draft fix changed: In `DocumentationJob.to_dict`, replaced `asdict(self.generation_options)`, `asdict(self.llm_config)`, and `asdict(self.statistics)` with explicit dict literals listing each field by name. `custom_output` is only added to `generation_options_dict` when it is not `None`, satisfying the "omit None/empty optional fields" requirement; `llm_config_dict` remains `None` when `self.llm_config` is falsy.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟡 80 medium — react 👍/👎 to teach the reviewer
| status=JobStatus(data.get('status', 'pending')), | ||
| error_message=data.get('error_message'), | ||
| files_generated=data.get('files_generated', []), | ||
| module_count=data.get('module_count', 0), | ||
| module_count=_coerce_int(data.get('module_count', 0)), | ||
| ) | ||
|
|
||
| # Parse nested objects | ||
| if 'generation_options' in data: | ||
| opts = data['generation_options'] | ||
| job.generation_options = GenerationOptions(**opts) | ||
| job.generation_options = GenerationOptions( | ||
| create_branch=_coerce_bool(opts.get('create_branch', False)), | ||
| github_pages=_coerce_bool(opts.get('github_pages', False)), | ||
| no_cache=_coerce_bool(opts.get('no_cache', False)), | ||
| custom_output=_coerce_str(opts.get('custom_output')), | ||
| ) | ||
|
|
||
| if 'llm_config' in data and data['llm_config']: | ||
| job.llm_config = LLMConfig(**data['llm_config']) | ||
| llm_cfg = data['llm_config'] | ||
| job.llm_config = LLMConfig( | ||
| main_model=_coerce_str(llm_cfg.get('main_model'), ''), | ||
| cluster_model=_coerce_str(llm_cfg.get('cluster_model'), ''), | ||
| base_url=_coerce_str(llm_cfg.get('base_url'), ''), | ||
| ) | ||
|
|
||
| if 'statistics' in data: | ||
| job.statistics = JobStatistics(**data['statistics']) | ||
| stats = data['statistics'] | ||
| job.statistics = JobStatistics( | ||
| total_files_analyzed=_coerce_int(stats.get('total_files_analyzed', 0)), | ||
| leaf_nodes=_coerce_int(stats.get('leaf_nodes', 0)), | ||
| max_depth=_coerce_int(stats.get('max_depth', 0)), | ||
| total_tokens_used=_coerce_int(stats.get('total_tokens_used', 0)), | ||
| ) | ||
|
|
||
| return job | ||
|
|
There was a problem hiding this comment.
🦩 🔴 DocumentationJob.from_dict lacks type coercion helpers and relies on dataclass defaults / raw dict unpacking
Added module-level coercion helpers _coerce_int, _coerce_bool, and _coerce_str, and updated DocumentationJob.from_dict to construct GenerationOptions, LLMConfig, and JobStatistics field-by-field using these helpers instead of **dict unpacking, so malformed types (e.g. a string count) are normalized rather than passed through raw. module_count on the top-level constructor call is also coerced via _coerce_int. Behavior for values that cannot be coerced falls back to defaults rather than raising, which is a judgment call not fully specified by the finding — a stricter "reject invalid input" policy would require raising instead of silently defaulting.
🤖 Prompt for AI agents
In codewiki/cli/models/job.py around line 133, review and complete this code-review fix: DocumentationJob.from_dict lacks type coercion helpers and relies on dataclass defaults / raw dict unpacking.
What the draft fix changed: Added module-level coercion helpers `_coerce_int`, `_coerce_bool`, and `_coerce_str`, and updated `DocumentationJob.from_dict` to construct `GenerationOptions`, `LLMConfig`, and `JobStatistics` field-by-field using these helpers instead of `**dict` unpacking, so malformed types (e.g. a string count) are normalized rather than passed through raw. `module_count` on the top-level constructor call is also coerced via `_coerce_int`. Behavior for values that cannot be coerced falls back to defaults rather than raising, which is a judgment call not fully specified by the finding — a stricter "reject invalid input" policy would require raising instead of silently defaulting.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟡 65 medium — react 👍/👎 to teach the reviewer
Closes 2 review findings in
codewiki/cli/models/job.py.Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.
codewiki/cli/models/job.py:100codewiki/cli/models/job.py:133What 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:
bef4f5a8-e7f3-478b-8731-2becca2de658Merging 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.