Skip to content

fix(CODEWIKI-001): CU-86akn96pk LLMConfig in job.py only models two roles (main, cluster), missing the mandatory fallback role - #92

Draft
flamingo[bot] wants to merge 1 commit into
mainfrom
ai-fix/codewiki-001-151b1524-90e36f49
Draft

flamingo[bot] wants to merge 1 commit into
mainfrom
ai-fix/codewiki-001-151b1524-90e36f49

Conversation

@flamingo

@flamingo flamingo Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

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.

# Fix confidence Finding Location
1 🔴 55 low — review closely LLMConfig in job.py only models two roles (main, cluster), missing the mandatory fallback role codewiki/cli/models/job.py:39

What 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-2d911418acb4

Merging 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)

…luster), missing the mandatory fallback role

@flamingo flamingo Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 What this fix changed, finding by finding

1 finding(s) fixed in this draft — 1 explained inline on the diff; 1 low-confidence hunk(s) need close review before merging.

max_token_field: str = "max_tokens"


@dataclass

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔴 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

@flamingo flamingo Bot changed the title fix(CODEWIKI-001): LLMConfig in job.py only models two roles (main, cluster), missing the mandatory fallback role fix(CODEWIKI-001): CU-86akn96pk LLMConfig in job.py only models two roles (main, cluster), missing the mandatory fallback role Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants