fix(CODEWIKI-001): CU-86akn96pk LLMConfig in job.py only models two roles (main, cluster), missing the mandatory fallback role - #92
Conversation
…luster), missing the mandatory fallback role
| max_token_field: str = "max_tokens" | ||
|
|
||
|
|
||
| @dataclass |
There was a problem hiding this comment.
🦩 🔴 LLMConfig in job.py only models two roles (main, cluster), missing the mandatory fallback role
Replaced the two-role LLMConfig (main_model, cluster_model, shared base_url) with a three-role model in codewiki/cli/models/job.py: added a new LLMRoleConfig dataclass carrying the full per-role field set (model, api_key, base_url, api_version, max_tokens, temperature, temperature_supported, max_token_field), and redefined LLMConfig to have cluster, main, and fallback fields of type LLMRoleConfig. Updated _coerce_llm_config to build all three roles via a new _coerce_llm_role_config helper, and updated DocumentationJob.to_dict to serialize each role's full field set under llm_config.cluster/main/fallback. This is a breaking change to the on-disk/JSON job schema and to any other code in the repo that reads llm_config.main_model / llm_config.cluster_model / llm_config.base_url directly — those call sites (outside this file, not visible here) will need to be updated to the new llm_config.main.model / llm_config.cluster.model / per-role base_url shape for the fix to be complete across the codebase.
🤖 Prompt for AI agents
In codewiki/cli/models/job.py around line 39, review and complete this code-review fix: LLMConfig in job.py only models two roles (main, cluster), missing the mandatory fallback role.
What the draft fix changed: Replaced the two-role `LLMConfig` (`main_model`, `cluster_model`, shared `base_url`) with a three-role model in `codewiki/cli/models/job.py`: added a new `LLMRoleConfig` dataclass carrying the full per-role field set (model, api_key, base_url, api_version, max_tokens, temperature, temperature_supported, max_token_field), and redefined `LLMConfig` to have `cluster`, `main`, and `fallback` fields of type `LLMRoleConfig`. Updated `_coerce_llm_config` to build all three roles via a new `_coerce_llm_role_config` helper, and updated `DocumentationJob.to_dict` to serialize each role's full field set under `llm_config.cluster/main/fallback`. This is a breaking change to the on-disk/JSON job schema and to any other code in the repo that reads `llm_config.main_model` / `llm_config.cluster_model` / `llm_config.base_url` directly — those call sites (outside this file, not visible here) will need to be updated to the new `llm_config.main.model` / `llm_config.cluster.model` / per-role `base_url` shape for the fix to be complete across the codebase.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.
fix confidence: 🔴 55 low — review closely — react 👍/👎 to teach the reviewer
Closes findings from rule CODEWIKI-001 — LLMConfig in job.py only models two roles (main, cluster), missing the mandatory fallback role.
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:39What 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)