Remove duplicate test fixtures and consolidate context_window tags - #1585
Merged
Merged
Conversation
Delete two non-functional context window evaluation tests: - 47_truncated_logs_context_window: Eval was broken and marked as skip=true - 160c_cpu_per_namespace_graph_with_global_truncation: Env var override doesn't work Both tests had skip: true in their test_case.yaml files and were not providing value to the test suite.
Add missing context_window tag to three tests that validate context window/truncation behavior: - 103_logs_transparency_default_limit: Tests log truncation transparency - 160a_cpu_per_namespace_graph: Tests handling of large Prometheus result sets - 160b_cpu_per_namespace_graph_with_prom_truncation: Tests query_response_size_limit All context_window related evals now have consistent tagging.
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
📂 Previous Runs📜 Run @ ef4501b (#22168967572)✅ Results of HolmesGPT evalsAutomatically triggered by commit ef4501b on branch Results of HolmesGPT evals
✅ Results of HolmesGPT evalsAutomatically triggered by commit b1f1596 on branch 📖 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: |
Contributor
|
✅ 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:21b087f
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:21b087f me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:21b087f
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:21b087fPatch 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:21b087fRobusta 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:21b087f |
Contributor
WalkthroughThe PR adds a "context_window" tag to three existing LLM test fixtures and removes two complete test case directories along with their associated configurations, Kubernetes manifests, and fixture files. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
aantn
enabled auto-merge (squash)
February 19, 2026 04:49
Contributor
🔬 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}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/llm/fixtures/test_ask_holmes/160b_cpu_per_namespace_graph_with_prom_truncation/test_case.yaml (1)
5-6:⚠️ Potential issue | 🟡 MinorAdding a tag to a permanently-skipped test may create misleading filter results.
This test is marked
skip: truewith askip_reasonstating it is "no longer relevant." Addingcontext_windowto it means that any tag-filtered run usingcontext_windowwill surface this fixture but still skip it silently, potentially obscuring coverage gaps. If the test is truly obsolete, consider removing it entirely (as was done for47_truncated_logs_context_windowand160c_cpu_per_namespace_graph_with_global_truncationin this same PR); if it still has value, theskipshould be lifted.Also applies to: 18-18
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/llm/fixtures/test_ask_holmes/160b_cpu_per_namespace_graph_with_prom_truncation/test_case.yaml` around lines 5 - 6, The test fixture "160b_cpu_per_namespace_graph_with_prom_truncation" is marked skip: true but was updated to include the context_window tag, which can mislead tag-filtered test runs; either remove this obsolete fixture entirely (as done for similar tests) or unskip it by removing skip: true and skip_reason so it runs with the context_window tag—locate the YAML for 160b_cpu_per_namespace_graph_with_prom_truncation and either delete the file or delete the skip: true/skip_reason entries to enable the test, ensuring tag-filtered executions reflect reality.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@tests/llm/fixtures/test_ask_holmes/160b_cpu_per_namespace_graph_with_prom_truncation/test_case.yaml`:
- Around line 5-6: The test fixture
"160b_cpu_per_namespace_graph_with_prom_truncation" is marked skip: true but was
updated to include the context_window tag, which can mislead tag-filtered test
runs; either remove this obsolete fixture entirely (as done for similar tests)
or unskip it by removing skip: true and skip_reason so it runs with the
context_window tag—locate the YAML for
160b_cpu_per_namespace_graph_with_prom_truncation and either delete the file or
delete the skip: true/skip_reason entries to enable the test, ensuring
tag-filtered executions reflect reality.
---
Duplicate comments:
In
`@tests/llm/fixtures/test_ask_holmes/160a_cpu_per_namespace_graph/test_case.yaml`:
- Line 12: The YAML contains an unverified test tag "context_window" which may
not be registered in pyproject.toml and will cause pytest collection failures;
open pyproject.toml, locate the pytest markers/allowed-tags section (the same
registration used by the verification script referenced in
103_logs_transparency_default_limit/test_case.yaml) and add or confirm the
"context_window" tag is listed there, then re-run the verification script to
ensure the tag is valid for the test fixture.
arikalon1
approved these changes
Feb 19, 2026
arikalon1
pushed a commit
that referenced
this pull request
Feb 21, 2026
…1585) ## Summary This PR removes duplicate test fixtures for ask_holmes tests and consolidates context_window-related test tags across the test suite. ## Key Changes - **Removed duplicate test fixtures:** - Deleted `47_truncated_logs_context_window/` directory containing test case, manifest, toolsets configuration, and fixture files for a logs context window truncation test - Deleted `160c_cpu_per_namespace_graph_with_global_truncation/` directory containing test case and toolsets configuration for a Prometheus metrics test with global truncation - **Added context_window tag to existing tests:** - Added `context_window` tag to `103_logs_transparency_default_limit` test case - Added `context_window` tag to `160a_cpu_per_namespace_graph` test case - Added `context_window` tag to `160b_cpu_per_namespace_graph_with_prom_truncation` test case ## Rationale The changes consolidate test coverage by removing redundant test fixtures while ensuring existing tests that validate context window behavior are properly tagged for organization and filtering purposes. https://claude.ai/code/session_01TceJLidFkDBiUpG1qmroPZ --------- Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Arik Alon <alon.arik@gmail.com>
moshemorad
pushed a commit
that referenced
this pull request
Feb 22, 2026
…1585) ## Summary This PR removes duplicate test fixtures for ask_holmes tests and consolidates context_window-related test tags across the test suite. ## Key Changes - **Removed duplicate test fixtures:** - Deleted `47_truncated_logs_context_window/` directory containing test case, manifest, toolsets configuration, and fixture files for a logs context window truncation test - Deleted `160c_cpu_per_namespace_graph_with_global_truncation/` directory containing test case and toolsets configuration for a Prometheus metrics test with global truncation - **Added context_window tag to existing tests:** - Added `context_window` tag to `103_logs_transparency_default_limit` test case - Added `context_window` tag to `160a_cpu_per_namespace_graph` test case - Added `context_window` tag to `160b_cpu_per_namespace_graph_with_prom_truncation` test case ## Rationale The changes consolidate test coverage by removing redundant test fixtures while ensuring existing tests that validate context window behavior are properly tagged for organization and filtering purposes. https://claude.ai/code/session_01TceJLidFkDBiUpG1qmroPZ --------- Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Mohse Morad <moshemorad12340@gmail.com>
moshemorad
pushed a commit
that referenced
this pull request
Feb 22, 2026
…1585) ## Summary This PR removes duplicate test fixtures for ask_holmes tests and consolidates context_window-related test tags across the test suite. ## Key Changes - **Removed duplicate test fixtures:** - Deleted `47_truncated_logs_context_window/` directory containing test case, manifest, toolsets configuration, and fixture files for a logs context window truncation test - Deleted `160c_cpu_per_namespace_graph_with_global_truncation/` directory containing test case and toolsets configuration for a Prometheus metrics test with global truncation - **Added context_window tag to existing tests:** - Added `context_window` tag to `103_logs_transparency_default_limit` test case - Added `context_window` tag to `160a_cpu_per_namespace_graph` test case - Added `context_window` tag to `160b_cpu_per_namespace_graph_with_prom_truncation` test case ## Rationale The changes consolidate test coverage by removing redundant test fixtures while ensuring existing tests that validate context window behavior are properly tagged for organization and filtering purposes. https://claude.ai/code/session_01TceJLidFkDBiUpG1qmroPZ --------- Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Mohse Morad <moshemorad12340@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR removes duplicate test fixtures for ask_holmes tests and consolidates context_window-related test tags across the test suite.
Key Changes
Removed duplicate test fixtures:
47_truncated_logs_context_window/directory containing test case, manifest, toolsets configuration, and fixture files for a logs context window truncation test160c_cpu_per_namespace_graph_with_global_truncation/directory containing test case and toolsets configuration for a Prometheus metrics test with global truncationAdded context_window tag to existing tests:
context_windowtag to103_logs_transparency_default_limittest casecontext_windowtag to160a_cpu_per_namespace_graphtest casecontext_windowtag to160b_cpu_per_namespace_graph_with_prom_truncationtest caseRationale
The changes consolidate test coverage by removing redundant test fixtures while ensuring existing tests that validate context window behavior are properly tagged for organization and filtering purposes.
https://claude.ai/code/session_01TceJLidFkDBiUpG1qmroPZ
Summary by CodeRabbit