Conversation
Signed-off-by: Codex <codex@openai.com>
WalkthroughThis PR removes the entire workload health feature from Holmes, including two API endpoints ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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 |
Signed-off-by: Codex <codex@openai.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/llm/conftest.py (1)
49-49: Remove unusedLLM_TEST_TYPESconstant.This constant is not used anywhere in the codebase. The
is_llm_test()function uses hardcoded checks instead of referencing it. Remove the constant to eliminate dead code.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
docs/reference/http-api.mdevals_per_branch.jsonholmes/core/conversations.pyholmes/core/models.pyholmes/plugins/prompts/kubernetes_workload_ask.jinja2holmes/plugins/prompts/kubernetes_workload_chat.jinja2server.pytests/core/test_prompt.pytests/llm/conftest.pytests/llm/fixtures/test_workload_health/01_crashpod/issue_data.jsontests/llm/fixtures/test_workload_health/01_crashpod/kubectl_describe.txttests/llm/fixtures/test_workload_health/01_crashpod/kubectl_get_by_kind_in_namespace.txttests/llm/fixtures/test_workload_health/01_crashpod/kubectl_logs_all_containers.txttests/llm/fixtures/test_workload_health/01_crashpod/resource_instructions.jsontests/llm/fixtures/test_workload_health/01_crashpod/test_case.yamltests/llm/fixtures/test_workload_health/01_crashpod/workload_health_request.jsontests/llm/test_workload_health.pytests/llm/utils/reporting/github_reporter.pytests/llm/utils/test_case_utils.pytests/test_ai_safety_prompt.pytests/test_server_endpoints.py
💤 Files with no reviewable changes (16)
- tests/llm/fixtures/test_workload_health/01_crashpod/kubectl_logs_all_containers.txt
- tests/llm/fixtures/test_workload_health/01_crashpod/kubectl_get_by_kind_in_namespace.txt
- tests/llm/fixtures/test_workload_health/01_crashpod/issue_data.json
- server.py
- tests/llm/fixtures/test_workload_health/01_crashpod/resource_instructions.json
- holmes/plugins/prompts/kubernetes_workload_chat.jinja2
- tests/test_ai_safety_prompt.py
- holmes/core/models.py
- tests/llm/fixtures/test_workload_health/01_crashpod/workload_health_request.json
- holmes/core/conversations.py
- tests/test_server_endpoints.py
- tests/llm/fixtures/test_workload_health/01_crashpod/kubectl_describe.txt
- holmes/plugins/prompts/kubernetes_workload_ask.jinja2
- tests/core/test_prompt.py
- tests/llm/test_workload_health.py
- tests/llm/fixtures/test_workload_health/01_crashpod/test_case.yaml
🧰 Additional context used
📓 Path-based instructions (3)
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
When writing documentation in the docs/ directory, always add a blank line between a header/bold text and a list, otherwise MkDocs won't render the list properly
Files:
docs/reference/http-api.md
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Use Ruff for formatting and linting (configured in pyproject.toml)
Type hints required (mypy configuration in pyproject.toml)
ALWAYS place Python imports at the top of the file, not inside functions or methods
Files:
tests/llm/conftest.pytests/llm/utils/reporting/github_reporter.pytests/llm/utils/test_case_utils.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Tests: match source structure under
tests/
Files:
tests/llm/conftest.pytests/llm/utils/reporting/github_reporter.pytests/llm/utils/test_case_utils.py
🔇 Additional comments (5)
tests/llm/conftest.py (3)
52-59: LGTM! Test type filtering correctly updated.The function now properly filters for only the remaining test types (
test_ask_holmesandtest_investigate), correctly excluding workload health tests.
877-878: LGTM! Documentation accurately reflects the changes.The comment has been correctly updated to reference only the remaining test types.
49-59: This review comment does not match the provided code snippet.The code at lines 49-59 of
tests/llm/conftest.pydefinesLLM_TEST_TYPESandis_llm_test()function, which are unrelated to workload health. Additionally, searching the codebase found noworkload_healthorWorkloadHealthreferences, making this verification request moot. Either the code snippet or file/line references are incorrect.Likely an incorrect or invalid review comment.
tests/llm/utils/test_case_utils.py (1)
17-17: LGTM! Import correctly updated.The removal of
WorkloadHealthRequestfrom the imports is correct and aligns with the broader removal of workload health functionality from the codebase.tests/llm/utils/reporting/github_reporter.py (1)
112-112: Workload health regression tracking correctly removed.The change cleanly removes
workload_health_regressionsfrom the regression count calculation. The function now correctly sums only the two active test types (ask_holmes_regressionsandinvestigate_regressions), matching the function's logic which processes only "ask_holmes" and "investigate" test results. Verification confirms no stale references to workload_health remain in the codebase.
|
|
||
| ## Overview | ||
| The HolmesGPT API provides endpoints for automated investigations, workload health checks, and conversational troubleshooting. This document describes each endpoint, its purpose, request fields, and example usage. | ||
| The HolmesGPT API provides endpoints for automated investigations and conversational troubleshooting. This document describes each endpoint, its purpose, request fields, and example usage. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l docs/reference/http-api.mdRepository: HolmesGPT/holmesgpt
Length of output: 93
🏁 Script executed:
head -50 docs/reference/http-api.md | cat -nRepository: HolmesGPT/holmesgpt
Length of output: 1976
🏁 Script executed:
# Check for markdown formatting issues (headers followed by lists without blank lines)
rg -n '^#+\s|^\*\s|^-\s|^\d+\.\s' docs/reference/http-api.md | head -30Repository: HolmesGPT/holmesgpt
Length of output: 1293
🏁 Script executed:
# Check for headers/bold text followed directly by lists without blank line
# Pattern: lines starting with # or ** followed by lines starting with - or *
python3 << 'EOF'
with open('docs/reference/http-api.md', 'r') as f:
lines = f.readlines()
issues = []
for i in range(len(lines) - 1):
current_line = lines[i].rstrip()
next_line = lines[i + 1].rstrip()
# Check if current line is a header or bold text
is_header = current_line.startswith('#')
is_bold = '**' in current_line
# Check if next line is a list item (with content, not blank)
is_list_item = next_line.strip().startswith('-') or next_line.strip().startswith('*') or (next_line.strip() and next_line.strip()[0].isdigit() and '.' in next_line)
if (is_header or is_bold) and is_list_item and next_line.strip():
issues.append((i + 1, current_line, i + 2, next_line))
if issues:
print("Found markdown formatting violations (header/bold followed directly by list):")
for line_num, current, next_num, next_content in issues:
print(f"\nLine {line_num}: {current[:80]}")
print(f"Line {next_num}: {next_content[:80]}")
else:
print("No markdown formatting violations found (headers/bold properly separated from lists)")
EOFRepository: HolmesGPT/holmesgpt
Length of output: 149
🏁 Script executed:
# Run the verification script from the review comment
# Verify no stale references to removed workload health endpoints
# Search documentation files for references to removed endpoints
echo "Checking documentation files..."
rg -n --type=md -C2 'workload_health_check|workload_health_chat' docs/
# Search Python code comments for references to removed endpoints
echo -e "\nChecking Python code comments..."
rg -n --type=py -C2 '/api/workload_health'
# Search for any remaining "workload health" or "workload_health" references in docs
echo -e "\nChecking for general workload health references in docs..."
rg -n --type=md 'workload.?health' docs/Repository: HolmesGPT/holmesgpt
Length of output: 188
🏁 Script executed:
# Check for markdown formatting violations more efficiently
rg -n '^(#{1,6}\s|[*_]{2}[^*_]+[*_]{2})' docs/reference/http-api.md | head -40Repository: HolmesGPT/holmesgpt
Length of output: 1589
🏁 Script executed:
# For each header or bold, check if the next non-empty line is a list
python3 << 'EOF'
with open('docs/reference/http-api.md', 'r') as f:
lines = f.readlines()
violations = []
for i in range(len(lines)):
current = lines[i].rstrip()
# Check if it's a header
if current.startswith('#'):
# Look for next non-empty line
j = i + 1
while j < len(lines) and not lines[j].strip():
j += 1
if j < len(lines):
next_line = lines[j].rstrip()
# Check if next non-empty line is a list item
stripped = next_line.strip()
if stripped.startswith('-') or stripped.startswith('*') or (stripped and stripped[0].isdigit() and '.' in stripped.split()[0]):
# There should be a blank line between header and list
if i + 1 == j: # No blank line
violations.append((i+1, current[:60], j+1, next_line[:60]))
if violations:
print("Markdown formatting violations found:")
for line_num, header, list_line, list_content in violations:
print(f"Line {line_num}: {header}")
print(f" -> Line {list_line}: {list_content}")
print()
else:
print("✓ No violations found - all headers/bold text properly separated from lists")
EOFRepository: HolmesGPT/holmesgpt
Length of output: 673
🏁 Script executed:
# More precise check for violations: headers/bold followed directly by actual list items
python3 << 'EOF'
import re
with open('docs/reference/http-api.md', 'r') as f:
lines = f.readlines()
violations = []
for i in range(len(lines) - 1):
current = lines[i].rstrip()
next_line = lines[i + 1].rstrip()
next_stripped = next_line.strip()
# Check if current line is header or contains bold text
is_header = current.startswith('#')
has_bold = '**' in current and not current.startswith('#')
# Check if NEXT line (immediately after) is a list item
is_list = (next_stripped.startswith('-') or
next_stripped.startswith('*') or
(next_stripped and re.match(r'^\d+\.', next_stripped)))
if (is_header or has_bold) and is_list and next_stripped:
violations.append((i+1, current[:70], i+2, next_line[:70]))
if violations:
print("Found violations (header/bold directly followed by list):")
for line_num, header, list_line, list_content in violations:
print(f"\nLine {line_num}: {header}")
print(f"Line {list_line}: {list_content}")
else:
print("✓ No violations found")
EOFRepository: HolmesGPT/holmesgpt
Length of output: 821
🏁 Script executed:
# Also check the specific area around line 4 mentioned in the review
sed -n '1,15p' docs/reference/http-api.md | cat -nRepository: HolmesGPT/holmesgpt
Length of output: 737
Add blank lines between headers/bold text and lists to comply with MkDocs rendering requirements.
The file violates the markdown formatting guideline at multiple locations. Headers and bold text must be separated from lists by a blank line, but the following sections have direct adjacency:
- Line 46-47:
###/api/chat(POST)→**Description:**(followed by content) - Line 85-86:
###/api/investigate(POST)→**Description:** - Line 145-146:
###/api/stream/investigate(POST)→**Description:** - Line 198-199:
###/api/issue_chat(POST)→**Description:** - Line 211-212:
**investigation_result** object:→-result(string, optional)... - Line 248-249:
###/api/model(GET)→**Description:**
Add blank lines after each header and bold section header to ensure proper MkDocs rendering.
🤖 Prompt for AI Agents
In docs/reference/http-api.md around the indicated lines (46-47, 85-86, 145-146,
198-199, 211-212, 248-249) the headers and bold section headers are immediately
followed by list content which breaks MkDocs rendering; fix by inserting a
single blank line after each affected header or bolded line so there is an empty
line between the header/bold text and the following list/content (i.e., add one
newline after those specific lines).
Signed-off-by: Codex <codex@openai.com>
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
closing, replaced by |
1 participant
Summary
Testing
Codex Task
Summary by CodeRabbit
Release Notes
/api/workload_health_checkendpoint/api/workload_health_chatendpoint✏️ Tip: You can customize this high-level summary in your review settings.