Remove additional instructions support - #1477
Conversation
|
📂 Previous Runs📜 Run @ 2650c91 (#21709914696)✅ Results of HolmesGPT evalsAutomatically triggered by commit 2650c91 on branch Results of HolmesGPT evals
📜 Run @ 0b070c9 (#21670750389)✅ Results of HolmesGPT evalsAutomatically triggered by commit 0b070c9 on branch Results of HolmesGPT evals
📜 Run @ 1727cce (#21670552574)✅ Results of HolmesGPT evalsAutomatically triggered by commit 1727cce on branch Results of HolmesGPT evals
📜 Run @ 1c1160b (#21666478936)✅ Results of HolmesGPT evalsAutomatically triggered by commit 1c1160b on branch Results of HolmesGPT evals
📜 Run @ c037393 (#21666237870)✅ Results of HolmesGPT evalsAutomatically triggered by commit c037393 on branch Results of HolmesGPT evals
✅ Results of HolmesGPT evalsAutomatically triggered by commit de6bead on branch Results of HolmesGPT evals
📖 Legend
🔄 Re-run evals manually
Option 1: Comment on this PR with Or with more options (one per line): Run evals on a different branch (e.g., master) for comparison:
Quick re-run: Use Option 2: Trigger via GitHub Actions UI → "Run workflow" 🏷️ Valid markers
Commands: CLI: |
|
✅ Docker image ready for
Use this tag to pull the image for testing. 📋 Copy commandsgcloud auth configure-docker us-central1-docker.pkg.dev
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:ea2aa9c
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:ea2aa9c me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:ea2aa9c
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:ea2aa9cPatch Helm values in one line (choose the chart you use): HolmesGPT chart: helm upgrade --install holmesgpt ./helm/holmes \
--set registry=me-west1-docker.pkg.dev/robusta-development/development \
--set image=holmes-dev:ea2aa9cRobusta wrapper chart: helm upgrade --install robusta robusta/robusta \
--reuse-values \
--set holmes.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.image=holmes-dev:ea2aa9c |
WalkthroughRemoved the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🔬 CLI Performance Benchmark🟡 Startup Time (no LLM)Measures
🟡 Full CLI with LLMMeasures
PR: |
9a01f55 to
d07a816
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@holmes/core/tools.py`:
- Around line 501-514: The logger currently emits the full contents of
self.additional_instructions (see the logger.info call near
enable_additional_instructions and the __apply_additional_instructions usage),
which may leak secrets; change the logging to avoid printing the instruction
string itself and instead log safe metadata such as the toolset name
(toolset_name), existence flag, and/or instruction length or a fixed
placeholder, and keep the call sites around enable_additional_instructions and
the error branch unchanged otherwise so output_with_instructions is produced by
__apply_additional_instructions when allowed.
c037393 to
8e3cb92
Compare
8e3cb92 to
1c1160b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@holmes/common/env_vars.py`:
- Around line 155-156: load_bool currently relies on json.loads which mis-parses
common boolean strings ("yes"/"no") and returns ints for "1"/"0", causing
type-safety issues for ENABLE_ADDITIONAL_INSTRUCTIONS; update load_bool to
accept and normalize common boolean representations (e.g., case-insensitive
"true"/"false", "yes"/"no", "1"/"0", and actual bools) and return Optional[bool]
consistently (None when unset/invalid), and add an explicit type annotation to
the constant as ENABLE_ADDITIONAL_INSTRUCTIONS: Optional[bool] =
load_bool("ENABLE_ADDITIONAL_INSTRUCTIONS", False).
🧹 Nitpick comments (1)
holmes/common/env_vars.py (1)
155-156: Add type annotation to maintain consistency with Python typing guidelines.The
load_bool()function returnsOptional[bool], so the suggested type annotation is correct. However, note that many similar flags in this file (e.g.,ENABLE_TELEMETRY,DEVELOPMENT_MODE,ROBUSTA_AI) also useload_bool()without type annotations. Consider applying this annotation across all module-level globals for consistency with the coding guideline requiring type hints throughout Python code.♻️ Suggested annotation
-ENABLE_ADDITIONAL_INSTRUCTIONS = load_bool("ENABLE_ADDITIONAL_INSTRUCTIONS", False) +ENABLE_ADDITIONAL_INSTRUCTIONS: Optional[bool] = load_bool( + "ENABLE_ADDITIONAL_INSTRUCTIONS", False +)
1c1160b to
1727cce
Compare
naomi-robusta
changed the title
4aa576d to
0b070c9
Compare
Signed-off-by: Naomi Caren <naomi@robusta.dev>
0b070c9 to
2650c91
Compare
3 participants
Remove additional_instructions support
Summary by CodeRabbit