chore: move utils functions out of main.py - #672
Conversation
WalkthroughThe changes refactor logging, result handling, and system prompt help text out of the main CLI module into dedicated utility modules. The main CLI file now imports these functionalities from Changes
Estimated code review effort2 (~15 minutes) ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (2)
holmes/utils/console/logging.py (2)
12-12: Consider removing or implementing the TODO comment.The
LOG_QUERIESverbosity level is marked as unused. Consider either implementing its functionality or removing it if it's not needed.
61-76: Simplify logging level configuration for VERBOSE mode.The current implementation sets the level to INFO and then immediately overrides it to DEBUG, which is redundant.
Consider this cleaner approach:
elif verbosity == Verbosity.VERBOSE: logging.basicConfig( - level=logging.INFO, + level=logging.DEBUG, format="%(message)s", handlers=[ RichHandler( show_level=False, markup=True, show_time=False, show_path=False, console=Console(width=None), ) ], ) - logging.getLogger().setLevel(logging.DEBUG) suppress_noisy_logs()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
holmes/main.py(1 hunks)holmes/utils/console/consts.py(1 hunks)holmes/utils/console/logging.py(1 hunks)holmes/utils/console/result.py(1 hunks)
🧠 Learnings (2)
📓 Common learnings
Learnt from: nherment
PR: robusta-dev/holmesgpt#610
File: .github/workflows/llm-evaluation.yaml:39-42
Timestamp: 2025-07-08T08:45:41.069Z
Learning: When suggesting improvements to environment variable handling in robusta-dev/holmesgpt, check first if validation logic already exists rather than reimplementing it.
holmes/main.py (2)
Learnt from: nherment
PR: #436
File: tests/llm/utils/mock_utils.py:240-249
Timestamp: 2025-06-05T12:23:27.634Z
Learning: The holmesgpt project uses Python >= 3.10 and prefers modern type hint syntax like list[str], dict[str, int] over importing equivalent types from the typing module like List[str], Dict[str, int].
Learnt from: nherment
PR: #535
File: holmes/plugins/toolsets/bash/bash_toolset.py:207-209
Timestamp: 2025-06-24T05:51:04.543Z
Learning: The init_config method in toolsets should be idempotent - safely callable multiple times without errors. self.config should maintain consistent typing (not alternate between dict and config object types) throughout the object lifecycle.
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nherment
PR: robusta-dev/holmesgpt#610
File: .github/workflows/llm-evaluation.yaml:39-42
Timestamp: 2025-07-08T08:45:41.069Z
Learning: When suggesting improvements to environment variable handling in robusta-dev/holmesgpt, check first if validation logic already exists rather than reimplementing it.
holmes/main.py (2)
Learnt from: nherment
PR: #436
File: tests/llm/utils/mock_utils.py:240-249
Timestamp: 2025-06-05T12:23:27.634Z
Learning: The holmesgpt project uses Python >= 3.10 and prefers modern type hint syntax like list[str], dict[str, int] over importing equivalent types from the typing module like List[str], Dict[str, int].
Learnt from: nherment
PR: #535
File: holmes/plugins/toolsets/bash/bash_toolset.py:207-209
Timestamp: 2025-06-24T05:51:04.543Z
Learning: The init_config method in toolsets should be idempotent - safely callable multiple times without errors. self.config should maintain consistent typing (not alternate between dict and config object types) throughout the object lifecycle.
⏰ 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: Pre-commit checks
- GitHub Check: llm_evals
- GitHub Check: build (3.11)
- GitHub Check: build (3.12)
- GitHub Check: build (3.10)
🔇 Additional comments (5)
holmes/utils/console/consts.py (1)
1-2: LGTM! Clean extraction of help text constant.The system prompt help text is well-structured and clearly explains the different ways to specify system prompts. Moving this to a dedicated constants module improves code organization.
holmes/utils/console/result.py (1)
11-37: LGTM! Well-structured result handling function.The function correctly handles both CLI and Slack output destinations with proper formatting. The use of
markup=Falsefor tool call output is a good practice to prevent interpretation of arbitrary text as Rich markup.holmes/utils/console/logging.py (1)
28-41: Excellent noise reduction strategy.The
suppress_noisy_logsfunction properly suppresses verbose logging from third-party libraries, which will improve the user experience by reducing log noise.holmes/main.py (2)
43-45: LGTM! Clean import of extracted utility functions.The imports correctly reference the new utility modules that contain the refactored functionality.
221-221: All extracted utility functions are used consistentlyAll occurrences of
init_logging,handle_result, andsystem_prompt_helpinholmes/main.pyare imported and invoked correctly, with no remaining local definitions detected:
init_loggingcalls at lines 221, 390, 470, 524, 618, 714, 800, 886, 942, 957handle_resultcalls at lines 332, 445system_prompt_helpreferences at multiple help entries- No
def init_logging,def handle_result,def cli_flags_to_verbosity, ordef suppress_noisy_logsin this fileLGTM.
|
@mainred can you explain? Why wasn't it importable before? |
|
Hey @aantn, what we want to achieve in az cli is like in the following PR |
Got it, thanks! Approved. |
pavangudiwada
changed the title
2 participants
This PR moves the utils functions out of main.go to make utils function importal.