Repository navigation
Add behavior_controls to /api/chat for controlling prompt components - #1479
Conversation
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
|
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📂 Previous Runs📜 Run @ ce5e5c6 (#21794250430)✅ Results of HolmesGPT evalsAutomatically triggered by commit ce5e5c6 on branch Results of HolmesGPT evals
📜 Run @ 632deb9 (#21677940072)✅ Results of HolmesGPT evalsAutomatically triggered by commit 632deb9 on branch Results of HolmesGPT evals
📜 Run @ 76da8af (#21677673666)✅ Results of HolmesGPT evalsAutomatically triggered by commit 76da8af on branch 📜 Run @ c4c095a (#21675502515)✅ Results of HolmesGPT evalsAutomatically triggered by commit c4c095a on branch Results of HolmesGPT evals
📜 Run @ 501b08d (#21675429138)✅ Results of HolmesGPT evalsAutomatically triggered by commit 501b08d on branch Results of HolmesGPT evals
✅ Results of HolmesGPT evalsAutomatically triggered by commit 1bf3202 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:d545924
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:d545924 me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:d545924
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:d545924Patch 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:d545924Robusta 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:d545924 |
WalkthroughAdds per-component prompt overrides: a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (ChatRequest)
participant Server as Server (server.py)
participant Conv as Conversations.build_chat_messages
participant Prompt as Prompt.build_prompts
participant Builders as Prompt.system/user builders
participant LLM as LLM
Client->>Server: POST /chat with behavior_controls
Server->>Server: map string keys → PromptComponent (warn on unknown)
Server->>Conv: build_chat_messages(..., prompt_component_overrides)
Conv->>Prompt: build_prompts(prompt_component_overrides)
Prompt->>Builders: is_component_enabled(component, overrides)
Builders-->>Prompt: system/user prompt fragments
Prompt-->>Conv: composed prompts
Conv->>LLM: send final messages
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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. No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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 |
🔬 CLI Performance Benchmark🟡 Startup Time (no LLM)Measures
🟡 Full CLI with LLMMeasures
PR: |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server.py (1)
362-381:⚠️ Potential issue | 🟠 MajorAdd unit coverage for behavior_controls mapping and unknown-key handling.
This is a new feature path (case-insensitive mapping, ignored unknown keys), but there’s no unit test proving it works end-to-end through/api/chat.As per coding guidelines, All new features require unit tests; new toolsets require integration tests; complex investigations require LLM evaluation tests.
🧹 Nitpick comments (1)
tests/core/test_prompt.py (1)
520-555: Add type hints to new test methods for mypy compliance.
Annotatemonkeypatchand return types (e.g.,-> None) in this class to satisfy the project’s type-checking requirement.As per coding guidelines, Use mypy for type checking with type hints required in all code.♻️ Example update (apply similarly to the rest of this class)
- def test_no_overrides_returns_env_var_result(self, monkeypatch): + def test_no_overrides_returns_env_var_result( + self, monkeypatch: pytest.MonkeyPatch + ) -> None:
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server.py (1)
360-382:⚠️ Potential issue | 🟠 MajorAdd unit tests for behavior_controls mapping behavior.
This is a new API feature; please cover case‑insensitive key mapping and unknown‑key ignore behavior to lock the contract. As per coding guidelines: tests/**/*.py: All new features require unit tests; new toolsets require integration tests; complex investigations require LLM evaluation tests.
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
…1479) ## Summary Adds a `behavior_controls` field to the `/api/chat` endpoint that allows clients to override prompt components at request time. The primary use case is disabling the TodoWrite planning phase for faster responses. ## Changes - **`holmes/core/models.py`**: Added `behavior_controls: Optional[Dict[str, bool]]` to `ChatRequest` - **`holmes/core/prompt.py`**: - Added `is_component_enabled()` function with precedence: env var > API override > default - Refactored `build_system_prompt` and `build_user_prompt` to use local closure pattern - **`holmes/core/conversations.py`**: Pass overrides through `build_chat_messages` - **`server.py`**: Convert string keys to `PromptComponent` enum with graceful handling of unknown keys - **`tests/core/test_prompt.py`**: Added tests for `is_component_enabled` ## Usage ```json { "ask": "What's wrong with my pod?", "behavior_controls": { "todowrite_instructions": false, "todowrite_reminder": false } } Notes - Keys are case-insensitive and map 1:1 to PromptComponent enum values - Unknown keys are ignored with a warning log (forward compatible) - Env var ENABLED_PROMPTS always takes precedence over API overrides <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Per-request controls to selectively enable/disable individual prompt components via a new request field; environment settings still take precedence and unknown keys are ignored with a warning. * New fast-mode CLI flag that disables specific prompt components for quicker runs and surfaces those choices through the interactive and non-interactive ask flows; applied overrides are logged. * **Tests** * Added tests covering per-component enablement, override precedence, and env-var interactions. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Tomer Keshet <tomer@robusta.dev> Signed-off-by: Mohse Morad <moshemorad12340@gmail.com>
2 participants
Summary
Adds a
behavior_controlsfield to the/api/chatendpoint that allows clients to override prompt components at request time. The primary use case is disabling the TodoWrite planning phase for faster responses.Changes
holmes/core/models.py: Addedbehavior_controls: Optional[Dict[str, bool]]toChatRequestholmes/core/prompt.py:is_component_enabled()function with precedence: env var > API override > defaultbuild_system_promptandbuild_user_promptto use local closure patternholmes/core/conversations.py: Pass overrides throughbuild_chat_messagesserver.py: Convert string keys toPromptComponentenum with graceful handling of unknown keystests/core/test_prompt.py: Added tests foris_component_enabledUsage
{ "ask": "What's wrong with my pod?", "behavior_controls": { "todowrite_instructions": false, "todowrite_reminder": false } } Notes - Keys are case-insensitive and map 1:1 to PromptComponent enum values - Unknown keys are ignored with a warning log (forward compatible) - Env var ENABLED_PROMPTS always takes precedence over API overrides <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Per-request controls to selectively enable/disable individual prompt components via a new request field; environment settings still take precedence and unknown keys are ignored with a warning. * New fast-mode CLI flag that disables specific prompt components for quicker runs and surfaces those choices through the interactive and non-interactive ask flows; applied overrides are logged. * **Tests** * Added tests covering per-component enablement, override precedence, and env-var interactions. <!-- end of auto-generated comment: release notes by coderabbit.ai -->