Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
flamingo wants to merge 1 commit into
base: main
Choose a base branch
from
+53 −9
Draft

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 53 additions & 9 deletions codewiki/cli/models/job.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,25 @@ class JobStatistics:
total_tokens_used: int = 0


@dataclass
class LLMRoleConfig:
"""LLM configuration for a single role (cluster, main, or fallback)."""
model: str = ""
api_key: str = ""
base_url: str = ""
api_version: str = ""
max_tokens: int = 0
temperature: float = 0.0
temperature_supported: bool = True
max_token_field: str = "max_tokens"


@dataclass

@flamingo flamingo Bot Sep 21, 2026

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

class LLMConfig:
"""LLM configuration for a job."""
main_model: str
cluster_model: str
base_url: str
cluster: LLMRoleConfig = field(default_factory=LLMRoleConfig)
main: LLMRoleConfig = field(default_factory=LLMRoleConfig)
fallback: LLMRoleConfig = field(default_factory=LLMRoleConfig)


def _coerce_job_status(value: Any, default: JobStatus = JobStatus.PENDING) -> JobStatus:
Expand Down Expand Up @@ -80,16 +93,34 @@ def _coerce_generation_options(value: Any) -> GenerationOptions:
)


def _coerce_llm_role_config(value: Any) -> LLMRoleConfig:
"""Coerce a raw dict into an LLMRoleConfig instance."""
if isinstance(value, LLMRoleConfig):
return value
if not value:
return LLMRoleConfig()
return LLMRoleConfig(
model=value.get('model', ''),
api_key=value.get('api_key', ''),
base_url=value.get('base_url', ''),
api_version=value.get('api_version', ''),
max_tokens=_coerce_int(value.get('max_tokens'), 0),
temperature=float(value.get('temperature', 0.0) or 0.0),
temperature_supported=bool(value.get('temperature_supported', True)),
max_token_field=value.get('max_token_field', 'max_tokens'),
)


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', ''),
cluster=_coerce_llm_role_config(value.get('cluster')),
main=_coerce_llm_role_config(value.get('main')),
fallback=_coerce_llm_role_config(value.get('fallback')),
)


Expand Down Expand Up @@ -163,6 +194,18 @@ def fail(self, error_message: str):

def to_dict(self) -> Dict[str, Any]:
"""Convert to dictionary for JSON serialization."""
def _role_dict(role: LLMRoleConfig) -> Dict[str, Any]:
return {
"model": role.model,
"api_key": role.api_key,
"base_url": role.base_url,
"api_version": role.api_version,
"max_tokens": role.max_tokens,
"temperature": role.temperature,
"temperature_supported": role.temperature_supported,
"max_token_field": role.max_token_field,
}

data = {
"job_id": self.job_id,
"repository_path": self.repository_path,
Expand All @@ -183,9 +226,9 @@ def to_dict(self) -> Dict[str, Any]:
"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,
"cluster": _role_dict(self.llm_config.cluster),
"main": _role_dict(self.llm_config.main),
"fallback": _role_dict(self.llm_config.fallback),
} if self.llm_config else None,
"statistics": {
"total_files_analyzed": self.statistics.total_files_analyzed,
Expand Down Expand Up @@ -230,3 +273,4 @@ def from_dict(cls, data: Dict[str, Any]) -> 'DocumentationJob':

return job