Repository navigation
[ROB-2379] fixed issue not using model list from cli - #1096
Conversation
Avi-Robusta
changed the title
WalkthroughChanges add optional Changes
Sequence DiagramsequenceDiagram
participant CLI as Command Handler
participant Config as Config Factory
participant LLM as LLM Core
participant Models as Models Store
CLI->>+CLI: Parse model parameter (new)
CLI->>+Config: create_console_issue_investigator(model_name=model)
Config->>+LLM: _get_llm(model_key=model_name)
LLM->>+LLM: _should_load_config_model()
LLM->>Models: Check if model exists
alt Model already loaded
Models-->>LLM: Model found
LLM-->>Config: Use existing model (return False)
else Model not loaded
LLM->>Models: Load and add new model
Models-->>LLM: Model loaded
end
LLM-->>Config: LLM instance
Config-->>CLI: IssueInvestigator ready
CLI->>CLI: Execute investigation with selected model
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
holmes/core/llm.py (1)
552-554: Clarify the comment for accuracy.The comment states "model already loaded from file," but
self._llmscan contain models from multiple sources (model list file, Robusta AI models, or previously loaded config models). The comment should be more generic to avoid confusion.Consider updating the comment to reflect the actual behavior:
if self._llms and self.config.model in self._llms: - # model already loaded from file + # model already exists in loaded models, skip re-loading return Falseholmes/config.py (2)
288-296: Consider consistent parameter naming across similar methods.The parameter is named
model_namehere, but other similar factory methods in this class usemodel(seecreate_agui_toolcalling_llmat line 302,create_toolcalling_llmat line 315, andcreate_issue_investigatorat line 328). This inconsistency may confuse users of the API.Consider renaming for consistency:
def create_console_toolcalling_llm( self, dal: Optional["SupabaseDal"] = None, refresh_toolsets: bool = False, tracer=None, - model_name: Optional[str] = None, + model: Optional[str] = None, ) -> "ToolCallingLLM": tool_executor = self.create_console_tool_executor(dal, refresh_toolsets) from holmes.core.tool_calling_llm import ToolCallingLLM return ToolCallingLLM( tool_executor, self.max_steps, - self._get_llm(tracer=tracer, model_key=model_name), + self._get_llm(tracer=tracer, model_key=model), )And update all call sites in
holmes/main.pyaccordingly.
349-368: Consider adding tracer support for consistency.The
create_console_issue_investigatormethod doesn't accept or pass atracerparameter to_get_llm, while the similarcreate_console_toolcalling_llmmethod (line 288) does support tracing. This inconsistency may limit observability for console-based issue investigations.If tracing should be supported, update the method signature and call:
def create_console_issue_investigator( - self, dal: Optional["SupabaseDal"] = None, model_name: Optional[str] = None + self, dal: Optional["SupabaseDal"] = None, model_name: Optional[str] = None, tracer=None ) -> "IssueInvestigator": all_runbooks = load_builtin_runbooks() for runbook_path in self.custom_runbooks: all_runbooks.extend(load_runbooks_from_file(runbook_path)) from holmes.core.runbooks import RunbookManager runbook_manager = RunbookManager(all_runbooks) tool_executor = self.create_console_tool_executor(dal=dal) from holmes.core.tool_calling_llm import IssueInvestigator return IssueInvestigator( tool_executor=tool_executor, runbook_manager=runbook_manager, max_steps=self.max_steps, - llm=self._get_llm(model_key=model_name), + llm=self._get_llm(model_key=model_name, tracer=tracer), cluster_name=self.cluster_name, )Then update call sites in
holmes/main.pyif tracing is needed for those commands.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
holmes/config.py(3 hunks)holmes/core/llm.py(1 hunks)holmes/main.py(8 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Use Ruff for formatting and linting (configured in pyproject.toml) for all Python code
Type hints are required; code should pass mypy (configured in pyproject.toml)
ALWAYS place Python imports at the top of the file, not inside functions or methods
Files:
holmes/main.pyholmes/core/llm.pyholmes/config.py
🧬 Code graph analysis (2)
holmes/main.py (1)
holmes/config.py (1)
create_console_issue_investigator(349-368)
holmes/config.py (1)
holmes/core/tool_calling_llm.py (1)
ToolCallingLLM(165-1013)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build (3.12)
- GitHub Check: build (3.11)
- GitHub Check: build (3.10)
- GitHub Check: llm_evals
- GitHub Check: build
🔇 Additional comments (1)
holmes/main.py (1)
264-264: LGTM! Model parameter propagation is correct.The changes consistently propagate the
modelparameter through all CLI commands to enable explicit model selection. The pattern is uniform acrossask,investigate alertmanager,investigate jira,investigate ticket,investigate github,investigate pagerduty, andinvestigate opsgeniecommands.Also applies to: 417-417, 546-546, 622-622, 663-663, 738-738, 822-822, 908-908
mainred
left a comment
mainred
left a comment
There was a problem hiding this comment.
Thanks for fixing the issue.
3 participants
you can now use model list from the cli to support advanced model configuration
example model list file
run:
export MODEL_LIST_FILE_LOCATION=/path/to/model_list.yamlthen you can run commands like
poetry run holmes ask "how many pods are available and list all kubernetes tools" --model=sonnet --no-interactiveor
poetry run holmes ask "how many pods are available and list all kubernetes tools" --model=azure-4o --no-interactive