57_wrong_namespace fix judge to allow get logs from the right namespace

[Sheeproid]

Summary by CodeRabbit

• Tests
  • Enhanced test validation with explicit expected outcomes.
  • Improved test reliability by adding pod readiness checks with timeout handling and debugging output before test execution.
  • Refined test case configuration for better stability.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

A test fixture file is updated to include pod readiness checks before running the test. The expected output is modified from a single string to a list of two distinct semantics, and the evaluation block is removed.

Changes

Cohort / File(s)	Summary
Test fixture configuration tests/llm/fixtures/test_ask_holmes/57_wrong_namespace/test_case.yaml	Modified expected_output from a single string to a two-item list of explicit semantics for namespace mismatch validation. Expanded before_test with a pod readiness wait script (kubectl polling up to 60 seconds) for the video-streamer pod in namespace test-57. Removed evaluation block.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Single file affected with straightforward test configuration changes
• Added kubectl readiness check is a standard wait pattern
• Expected output modifications are clear and localized to test expectations

Possibly related PRs

• fix setups on logging evals #737: Modifies test_case.yaml files for Kubernetes namespace handling and pod readiness setup.
• isolate evals in separate namespaces so cleanup/teardown for one does… #600: Updates test fixtures to manage Kubernetes namespaces and pod waiting logic for test isolation.

Suggested reviewers

• aantn
• moshemorad
• pavangudiwada

Pre-merge checks

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: fixing the judge logic for the 57_wrong_namespace test case to allow retrieving logs from the correct namespace.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between af8b2a5 and 3bbd281.

📒 Files selected for processing (1)

• tests/llm/fixtures/test_ask_holmes/57_wrong_namespace/test_case.yaml

🧰 Additional context used

📓 Path-based instructions (3)

tests/**

📄 CodeRabbit inference engine (CLAUDE.md)

Tests: Match source structure under tests/

Files:

• tests/llm/fixtures/test_ask_holmes/57_wrong_namespace/test_case.yaml

tests/llm/**/*.yaml

📄 CodeRabbit inference engine (CLAUDE.md)

tests/llm/**/*.yaml: ALWAYS use Secrets for scripts in eval tests, not inline manifests or ConfigMaps, to prevent code visibility with kubectl describe
All pod names must be unique across LLM tests - never reuse pod names between tests
Never use resource names that hint at the problem or expected behavior in evals - use neutral names that don't give away what the LLM should discover

Files:

• tests/llm/fixtures/test_ask_holmes/57_wrong_namespace/test_case.yaml

tests/llm/**

📄 CodeRabbit inference engine (CLAUDE.md)

tests/llm/**: Each LLM test must use a dedicated namespace app- to prevent conflicts when tests run simultaneously
No fake/obvious logs in eval scenarios - avoid logs like 'Memory usage stabilized at 800MB'
No hints in eval filenames - use realistic names like 'training_pipeline.py' instead of 'disk_consumer.py'
Implement full architecture even if complex in evals (e.g., use Loki for log aggregation properly, not simplified alternatives)

Files:

• tests/llm/fixtures/test_ask_holmes/57_wrong_namespace/test_case.yaml

🧠 Learnings (5)

📚 Learning: 2025-12-21T13:17:48.366Z

Learnt from: CR
Repo: HolmesGPT/holmesgpt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T13:17:48.366Z
Learning: Applies to tests/llm/**/*.yaml : Never use resource names that hint at the problem or expected behavior in evals - use neutral names that don't give away what the LLM should discover

Applied to files:

• tests/llm/fixtures/test_ask_holmes/57_wrong_namespace/test_case.yaml

📚 Learning: 2025-12-21T13:17:48.366Z

Learnt from: CR
Repo: HolmesGPT/holmesgpt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T13:17:48.366Z
Learning: Applies to tests/llm/**/*.yaml : ALWAYS use Secrets for scripts in eval tests, not inline manifests or ConfigMaps, to prevent code visibility with kubectl describe

Applied to files:

• tests/llm/fixtures/test_ask_holmes/57_wrong_namespace/test_case.yaml

📚 Learning: 2025-12-21T13:17:48.366Z

Learnt from: CR
Repo: HolmesGPT/holmesgpt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T13:17:48.366Z
Learning: Applies to tests/llm/**/*.yaml : All pod names must be unique across LLM tests - never reuse pod names between tests

Applied to files:

• tests/llm/fixtures/test_ask_holmes/57_wrong_namespace/test_case.yaml

📚 Learning: 2025-12-21T13:17:48.366Z

Learnt from: CR
Repo: HolmesGPT/holmesgpt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T13:17:48.366Z
Learning: Applies to tests/llm/** : Each LLM test must use a dedicated namespace app-<testid> to prevent conflicts when tests run simultaneously

Applied to files:

• tests/llm/fixtures/test_ask_holmes/57_wrong_namespace/test_case.yaml

📚 Learning: 2025-12-21T13:17:48.366Z

Learnt from: CR
Repo: HolmesGPT/holmesgpt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T13:17:48.366Z
Learning: Applies to tests/llm/** : No fake/obvious logs in eval scenarios - avoid logs like 'Memory usage stabilized at 800MB'

Applied to files:

• tests/llm/fixtures/test_ask_holmes/57_wrong_namespace/test_case.yaml

⏰ 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 (4)

tests/llm/fixtures/test_ask_holmes/57_wrong_namespace/test_case.yaml (4)

3-5: LGTM! Expected output properly structured for namespace mismatch test.

The two-part requirement correctly validates that the LLM both recognizes the namespace mismatch and successfully locates the resource in the correct namespace.

---

6-26: LGTM! Pod readiness check properly implemented.

The before_test script correctly waits for the pod to be ready with appropriate timeout (60s), clear debugging output, and proper error handling. This prevents race conditions where the test might run before the pod is ready.

---

7-7: Move inline scripts to a Secret to prevent visibility with kubectl describe.

The manifest.yaml violates the coding guideline by embedding shell scripts directly in the Deployment spec at lines 25-36. Per requirements, all scripts in eval tests must use Secrets, not inline manifests, to prevent code exposure with kubectl describe pod. Extract the command/args into a Secret and mount it as a volume or reference it appropriately.

Additionally, review the namespace naming: the test uses test-57 instead of the required app-<testid> pattern (app-57), even though the namespace mismatch appears intentional for this test scenario.

⛔ Skipped due to learnings

Learnt from: CR
Repo: HolmesGPT/holmesgpt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T13:17:48.366Z
Learning: Applies to tests/llm/**/*.yaml : ALWAYS use Secrets for scripts in eval tests, not inline manifests or ConfigMaps, to prevent code visibility with kubectl describe

Learnt from: CR
Repo: HolmesGPT/holmesgpt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T13:17:48.366Z
Learning: Applies to tests/llm/**/*.yaml : All pod names must be unique across LLM tests - never reuse pod names between tests

Learnt from: CR
Repo: HolmesGPT/holmesgpt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T13:17:48.366Z
Learning: Applies to tests/llm/**/*.yaml : Never use resource names that hint at the problem or expected behavior in evals - use neutral names that don't give away what the LLM should discover

---

1-2: Fix namespace naming to follow coding guidelines.

This test uses test-57 namespace, which deviates from the required app-<testid> pattern. While the namespace is unique and the deviation is intentional to test namespace mismatch detection, the coding guidelines explicitly require the app-<testid> pattern for all LLM tests. Change the namespace to app-57 to comply with the guidelines.

⛔ Skipped due to learnings

Learnt from: CR
Repo: HolmesGPT/holmesgpt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T13:17:48.366Z
Learning: Applies to tests/llm/** : Each LLM test must use a dedicated namespace app-<testid> to prevent conflicts when tests run simultaneously

Learnt from: CR
Repo: HolmesGPT/holmesgpt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T13:17:48.366Z
Learning: Applies to tests/llm/**/*.yaml : All pod names must be unique across LLM tests - never reuse pod names between tests

Learnt from: CR
Repo: HolmesGPT/holmesgpt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T13:17:48.366Z
Learning: Applies to tests/llm/**/*.yaml : Never use resource names that hint at the problem or expected behavior in evals - use neutral names that don't give away what the LLM should discover

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

Dev Docker images are ready for this commit:

• us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:3bbd281e206d36709de4002bff2bc4825f360e71
• us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:3bbd281

Use either tag to pull the image for testing.

[github-actions]

Results of HolmesGPT evals

• ask_holmes: 32/37 test cases were successful, 1 regressions, 1 setup failures, 3 mock failures

Test suite	Test case	Status
ask	01_how_many_pods	✅
ask	02_what_is_wrong_with_pod	✅
ask	04_related_k8s_events	✅
ask	05_image_version	✅
ask	09_crashpod	✅
ask	10_image_pull_backoff	✅
ask	110_k8s_events_image_pull	✅
ask	11_init_containers	✅
ask	13a_pending_node_selector_basic	✅
ask	14_pending_resources	✅
ask	15_failed_readiness_probe	✅
ask	163_compaction_follow_up	✅
ask	17_oom_kill	✅
ask	18_oom_kill_from_issues_history	✅
ask	19_detect_missing_app_details	✅
ask	20_long_log_file_search	✅
ask	24_misconfigured_pvc	❌
ask	24a_misconfigured_pvc_basic	✅
ask	28_permissions_error	🚧
ask	39_failed_toolset	✅
ask	41_setup_argo	✅
ask	42_dns_issues_steps_new_tools	✅
ask	43_current_datetime_from_prompt	✅
ask	45_fetch_deployment_logs_simple	✅
ask	51_logs_summarize_errors	✅
ask	53_logs_find_term	✅
ask	54_not_truncated_when_getting_pods	✅
ask	59_label_based_counting	✅
ask	60_count_less_than	✅
ask	61_exact_match_counting	✅
ask	63_fetch_error_logs_no_errors	✅
ask	79_configmap_mount_issue	✅
ask	83_secret_not_found	✅
ask	86_configmap_like_but_secret	✅
ask	93_calling_datadog[0]	🔧
ask	93_calling_datadog[1]	🔧
ask	93_calling_datadog[2]	🔧

Legend

• ✅ the test was successful
• :minus: the test was skipped
• ⚠️ the test failed but is known to be flaky or known to fail
• 🚧 the test had a setup failure (not a code regression)
• 🔧 the test failed due to mock data issues (not a code regression)
• 🚫 the test was throttled by API rate limits/overload
• ❌ the test failed and should be fixed before merging the PR