Repository navigation
Conversation
- Add new eval test 195_confluence_page_fetch that: - Creates a Confluence page with unique verification code in before_test - Tests that LLM can read the page ID file and fetch page contents - Deletes the test page in after_test - Add Atlassian Cloud passthrough to conftest.py for HTTP mocking - Add confluence marker to pyproject.toml Required environment variables: - CONFLUENCE_BASE_URL - CONFLUENCE_USER - CONFLUENCE_API_KEY - CONFLUENCE_SPACE_KEY Signed-off-by: Claude <noreply@anthropic.com>
Confluence toolset changes: - Add list_confluence_spaces tool to discover available spaces - Add get_confluence_space_pages tool to list pages in a space - Switch from Basic Auth to Bearer token authentication - Remove CONFLUENCE_USER requirement (only API key needed now) - Add configurable SSL verification via verify_ssl config option - Add explicit parameters with descriptions for all tools Eval test improvements: - Simplify test to use expected_output for secret verification - The LLM discovers the page using the new space/pages tools - Verification code in expected_output is invisible to LLM Documentation: - Add CLAUDE.md section explaining expected_output invisibility pattern - Document how to use verification codes for hallucination-proof tests Signed-off-by: Claude <noreply@anthropic.com>
Set default CONFLUENCE_BASE_URL and CONFLUENCE_SPACE_KEY via test_env_vars so they're available to both before_test scripts and the toolset: - CONFLUENCE_BASE_URL: https://robusta-llm-test.atlassian.net - CONFLUENCE_SPACE_KEY: MFS Only CONFLUENCE_API_KEY is required from the user (no default for secrets). Signed-off-by: Claude <noreply@anthropic.com>
Use bash defaults for CONFLUENCE_BASE_URL and CONFLUENCE_SPACE_KEY in before_test/after_test scripts. Note: The toolset still requires these env vars to be set since it uses shell variable expansion in curl commands. Required env vars: - CONFLUENCE_API_KEY (required, no default) - CONFLUENCE_BASE_URL (required for toolset, defaults in scripts) Signed-off-by: Claude <noreply@anthropic.com>
Signed-off-by: Claude <noreply@anthropic.com>
Replace YAML/curl-based toolset with Python implementation: - ConfluenceConfig with url, api_key, verify_ssl settings - Bearer token authentication - Health check on prerequisite validation - Three tools: - list_confluence_spaces: List all spaces - get_confluence_space_pages: Get pages in a space (with title filter) - fetch_confluence_page: Fetch page by ID Update eval toolsets.yaml to use config with env var references. Signed-off-by: Claude <noreply@anthropic.com>
The Python toolset now supports two authentication methods: 1. Basic Auth (for Atlassian Cloud): - Set username + api_key - Uses -u username:api_key format 2. Bearer Token (for self-hosted/PAT): - Set only api_key (no username) - Uses Authorization: Bearer header Update eval to use Basic Auth with CONFLUENCE_USERNAME env var. Signed-off-by: Claude <noreply@anthropic.com>
- Import and register ConfluenceToolset in load_python_toolsets - Update 195_confluence_page_fetch eval to disable K8s toolsets - Test passes successfully with verification code discovery Signed-off-by: Claude <noreply@anthropic.com>
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
📂 Previous Runs📜 Run @ 974da40 (#21149948617)✅ Results of HolmesGPT evalsAutomatically triggered by commit 974da40 on branch Results of HolmesGPT evals
Time/Cost columns show % change vs historical average (↑slower/costlier, ↓faster/cheaper). Changes under 10% shown as ±0%. Historical Comparison DetailsFilter: excluding branch 'claude/confluence-page-eval-By6aw' Status: Success - 11 test/model combinations loaded Experiments compared (30):
Comparison indicators:
📜 Run @ fb675df (#21149257179)✅ Results of HolmesGPT evalsAutomatically triggered by commit fb675df on branch Results of HolmesGPT evals
Time/Cost columns show % change vs historical average (↑slower/costlier, ↓faster/cheaper). Changes under 10% shown as ±0%. Historical Comparison DetailsFilter: excluding branch 'claude/confluence-page-eval-By6aw' Status: Success - 11 test/model combinations loaded Experiments compared (30):
Comparison indicators:
📜 Run @ a970171 (#21149130639)✅ Results of HolmesGPT evalsAutomatically triggered by commit a970171 on branch Results of HolmesGPT evals
Time/Cost columns show % change vs historical average (↑slower/costlier, ↓faster/cheaper). Changes under 10% shown as ±0%. Historical Comparison DetailsFilter: excluding branch 'claude/confluence-page-eval-By6aw' Status: Success - 10 test/model combinations loaded Experiments compared (30):
Comparison indicators:
📜 Run @ 2e3285f (#21148827383)✅ Results of HolmesGPT evalsAutomatically triggered by commit 2e3285f on branch Results of HolmesGPT evals
Time/Cost columns show % change vs historical average (↑slower/costlier, ↓faster/cheaper). Changes under 10% shown as ±0%. Historical Comparison DetailsFilter: excluding branch 'claude/confluence-page-eval-By6aw' Status: Success - 10 test/model combinations loaded Experiments compared (30):
Comparison indicators:
📜 Run @ 2e9377c (#21148560035)✅ Results of HolmesGPT evalsAutomatically triggered by commit 2e9377c on branch Results of HolmesGPT evals
Time/Cost columns show % change vs historical average (↑slower/costlier, ↓faster/cheaper). Changes under 10% shown as ±0%. Historical Comparison DetailsFilter: excluding branch 'claude/confluence-page-eval-By6aw' Status: Success - 10 test/model combinations loaded Experiments compared (30):
Comparison indicators:
✅ Results of HolmesGPT evalsAutomatically triggered by commit 67c52ba on branch Results of HolmesGPT evals
Time/Cost columns show % change vs historical average (↑slower/costlier, ↓faster/cheaper). Changes under 10% shown as ±0%. Historical Comparison DetailsFilter: excluding branch 'claude/confluence-page-eval-By6aw' Status: Success - 28 test/model combinations loaded Experiments compared (30):
Comparison indicators:
📖 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:b1a2c7c
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:b1a2c7c me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:b1a2c7c
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:b1a2c7cPatch 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:b1a2c7cRobusta 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:b1a2c7c |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughAdds a new Confluence Python toolset (client, auth, health checks, three tools), registers and exports it, removes the YAML toolset config, adds E2E Confluence test fixtures and passthroughs, extends OpenRouter support in classifiers, and updates CLAUDE.md and pytest markers. Changes
Sequence Diagram(s)sequenceDiagram
participant LLM as LLM / Holmes
participant Tool as Confluence Tool
participant Toolset as ConfluenceToolset
participant API as Confluence REST API
LLM->>Tool: _invoke(params, context)
Tool->>Toolset: _make_request(method, endpoint, params)
Toolset->>Toolset: _get_headers() / _get_auth()
Toolset->>API: HTTP request (auth, timeout, verify_ssl)
alt 200 OK
API-->>Toolset: JSON response
Toolset-->>Tool: StructuredToolResult(success, data)
Tool-->>LLM: StructuredToolResult(success)
else HTTP/auth/timeout/error
API-->>Toolset: error / no response
Toolset-->>Tool: StructuredToolResult(error with context)
Tool-->>LLM: StructuredToolResult(error)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
|
/eval |
|
@aantn Your eval run has finished. ✅ Completed successfully 🧪 Manual Eval Results
Results of HolmesGPT evals
Time/Cost columns show % change vs historical average (↑slower/costlier, ↓faster/cheaper). Changes under 10% shown as ±0%. Historical Comparison DetailsFilter: excluding branch 'master' Status: Success - 10 test/model combinations loaded Experiments compared (30):
Comparison indicators:
📖 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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@holmes/plugins/toolsets/confluence/confluence.py`:
- Around line 267-279: The linter flags unused arguments (ARG002); fix by
renaming unused parameters with a leading underscore: in the Confluence plugin
class rename the unused "context" parameter in _invoke to "_context" and rename
any other unused "params" or "context" parameters (for example in
get_parameterized_one_liner and the other similar methods flagged) to
"_params"/"_context" respectively, or alternatively add a per-argument noqa for
ARG002 if you prefer; ensure call sites are unaffected (only rename parameters
in the function signatures where the argument is not used).
- Around line 68-95: The catch-all Exception handlers in prerequisites_callable
and _perform_health_check should be narrowed and their error formatting updated
to satisfy Ruff: replace the broad except Exception in prerequisites_callable
with the specific validation errors thrown by ConfluenceConfig (e.g.,
pydantic.error_wrappers.ValidationError and TypeError) and update its message to
use an explicit f-string conversion like {e!r}; similarly, in
_perform_health_check replace the final except Exception with
requests.RequestException (or another specific requests exception class) and
change f"Confluence health check failed: {str(e)}" to f"Confluence health check
failed: {e!r}" so you avoid broad Exception catches and eliminate str(e) in
f-strings while keeping the existing logic in prerequisites_callable and
_perform_health_check.
- Around line 267-331: The list endpoints should return a NO_DATA
StructuredToolResult with the search context when they yield no items: update
the _invoke implementations (the top-level list spaces _invoke and
GetConfluenceSpacePages._invoke) to call the existing request, inspect the
response for an empty result set (e.g., no "results" or empty list), and if
empty return a StructuredToolResult with status NO_DATA and a message describing
the endpoint and the applied filters/query_params (include endpoint string and
relevant params such as "limit", "start", "type" or "space_key"/"title");
otherwise return the normal successful result from _make_request. Ensure the
message explicitly names the endpoint and the filter values so callers know what
was searched.
- Around line 180-234: In _make_request, update the error branches that build
StructuredToolResult (especially the requests.exceptions.HTTPError handler, and
also Timeout/ConnectionError/Exception branches) to include the full request
context (pass both params and query_params and body contents into the
StructuredToolResult.params or a new context field) and include the full API
error payload and HTTP status (use e.response.text and e.response.status_code
without truncation and include any parsed json) in the error string (or
StructuredToolResult.error payload) so the tool returns exact invocation +
query/body + full response text; ensure you reference the existing
StructuredToolResult and StructuredToolResultStatus constructors used in
_make_request to carry the extra fields.
🧹 Nitpick comments (3)
tests/llm/fixtures/test_ask_holmes/195_confluence_page_fetch/test_case.yaml (3)
74-76: Usejqfor robust JSON parsing.The current grep/cut approach for extracting
PAGE_IDis fragile and may fail if the JSON response contains multiple"id"fields or has different formatting. Usejqfor reliable JSON extraction.♻️ Suggested fix
- # Extract the page ID from the response - PAGE_ID=$(echo "$RESPONSE" | grep -o '"id":"[^"]*"' | head -1 | cut -d'"' -f4) + # Extract the page ID from the response + PAGE_ID=$(echo "$RESPONSE" | jq -r '.id')
83-84: Consider using test-specific temp file path for parallel test safety.Using
/tmp/confluence_test_page_idcould cause conflicts if multiple Confluence tests run in parallel. Consider using a test-specific path.♻️ Suggested fix
- # Save the page ID for cleanup - echo "$PAGE_ID" > /tmp/confluence_test_page_id + # Save the page ID for cleanup (test-specific path) + echo "$PAGE_ID" > /tmp/confluence_test_page_id_195And update
after_testcorrespondingly:- if [ -f /tmp/confluence_test_page_id ]; then - PAGE_ID=$(cat /tmp/confluence_test_page_id) + if [ -f /tmp/confluence_test_page_id_195 ]; then + PAGE_ID=$(cat /tmp/confluence_test_page_id_195) ... - rm -f /tmp/confluence_test_page_id + rm -f /tmp/confluence_test_page_id_195
1-13: Consider addingrunbooks: {}andinclude_tool_calls: true.Per the coding guidelines, eval tests should add
runbooks: {}for an empty runbook catalog. Additionally, while the verification code is specific, addinginclude_tool_calls: trueprovides extra assurance that the Confluence tools were actually called.♻️ Suggested additions
user_prompt: | Find and fetch the contents of the test page titled "HolmesGPT Eval Test Page" in Confluence. Tell me the verification code found in the page content. expected_output: - "Must report the verification code: HOLMES-EVAL-7x9k2m4p" include_tool_calls: true runbooks: {} tags: - confluence - question-answer - medium setup_timeout: 120
LiteLLM handles the openrouter/ prefix automatically, but the classifier uses the OpenAI client directly. Added prefix stripping and default base URL so CLASSIFIER_MODEL=openrouter/openai/gpt-4.1 works correctly with just OPENROUTER_API_KEY set. Signed-off-by: Claude <noreply@anthropic.com>
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)
tests/llm/utils/classifiers.py (1)
44-61: Misleading error message in else branch.The
elsebranch at lines 58-61 is reachable whenAZURE_API_KEYis set butAZURE_API_BASEis not. In this case, the error message "No API key found" is misleading because an API key is set — the actual problem is the missingAZURE_API_BASE.Proposed fix
elif OPENROUTER_API_KEY: client_api_key = OPENROUTER_API_KEY client_base_url = OPENROUTER_API_BASE or "https://openrouter.ai/api/v1" else: - raise ValueError( - "No API key found (AZURE_API_KEY, OPENAI_API_KEY, or OPENROUTER_API_KEY)" - ) + # This branch is reached when AZURE_API_KEY is set but AZURE_API_BASE is not + raise ValueError( + "AZURE_API_KEY is set but AZURE_API_BASE is missing. " + "Either set AZURE_API_BASE or use OPENAI_API_KEY/OPENROUTER_API_KEY instead." + )
|
/eval |
|
@aantn Your eval run has finished. ✅ Completed successfully 🧪 Manual Eval Results
Results of HolmesGPT evals
Historical comparison unavailable: No experiments found (excluding branch 'master') Historical Comparison DetailsFilter: excluding branch 'master' Status: No experiments found (excluding branch 'master') Comparison indicators:
📖 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: |
- Fix ARG002 linting: rename unused context params to _context - Add NO_DATA status for empty list results with search context - Include full request context in error responses (endpoint, query_params, body, HTTP status, response text, parsed JSON) - Better error messages with endpoint and filter details Signed-off-by: Claude <noreply@anthropic.com>
Keep original parameter names (context, params) for API consistency and add noqa: ARG002 comments to suppress linting warnings. Signed-off-by: Claude <noreply@anthropic.com>
- Auto-discover cloud_id using public /_edge/tenant_info endpoint - Use API gateway (api.atlassian.com) for Atlassian Cloud instances - Support on-premise Confluence with direct URL fallback - Update eval test to use API gateway for page creation/deletion - Add helpful error messages for service account permission issues The tenant_info endpoint works without authentication, which solves the chicken-and-egg problem for service accounts that can only access the API gateway but need cloud_id to construct the gateway URL. Signed-off-by: Claude <noreply@anthropic.com>
Signed-off-by: Claude <noreply@anthropic.com>
Service accounts use Bearer auth (no username), while personal tokens use Basic Auth (with username). Updated docstring to document both authentication methods clearly. Signed-off-by: Claude <noreply@anthropic.com>
Signed-off-by: Claude <noreply@anthropic.com>
Signed-off-by: Claude <noreply@anthropic.com>
Signed-off-by: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@holmes/plugins/toolsets/confluence/confluence.py`:
- Around line 98-122: In _discover_cloud_id, replace the broad "except
Exception" with explicit exception handling for requests.RequestException (for
network/HTTP issues) and ValueError (for response.json() parse errors), and
update the logging call to use structured logging (e.g., logging.debug("Could
not auto-discover cloud_id", exc_info=e) or logging.debug("Could not
auto-discover cloud_id: %s", e)) so errors aren't masked and are logged
consistently; keep the rest of the request/response logic and return None on
handled failures.
- Around line 295-303: The try/except around parsing e.response.json() uses a
bare except which hides errors; change it to catch ValueError (or
json.JSONDecodeError) specifically and log the parse failure instead of silently
passing—update the block where error_json is set (variable error_json and the
call e.response.json()) to use except ValueError as parse_err:
logger.debug/exception("Failed to parse error response JSON",
exc_info=parse_err) (or the module's configured logger) so callers still get
detailed error context.
♻️ Duplicate comments (1)
holmes/plugins/toolsets/confluence/confluence.py (1)
270-342: Include full request context in error payloads, not justdata.The
errorfield still omits query/body filters; LLMs may miss search context unless they inspectdata. Include the exact invocation + filters in the error payload (or params) for all error branches.As per coding guidelines, error messages should include the exact query/filters and full API error context.🔧 Proposed fix
request_context = { "endpoint": endpoint, "method": method, "query_params": query_params or {}, "body": body, "params": params, } + params_payload = {"tool_params": params, "request_context": request_context} @@ return StructuredToolResult( status=StructuredToolResultStatus.ERROR, - error=error_msg, + error=f"{error_msg} | request_context={request_context}", data=error_detail, - params=params, + params=params_payload, ) @@ return StructuredToolResult( status=StructuredToolResultStatus.ERROR, - error=f"Confluence request timed out for endpoint '{endpoint}' (timeout: {timeout or self._toolset.confluence_config.timeout}s)", + error=f"Confluence request timed out for endpoint '{endpoint}' (timeout: {timeout or self._toolset.confluence_config.timeout}s) | request_context={request_context}", data={"request_context": request_context}, - params=params, + params=params_payload, ) @@ return StructuredToolResult( status=StructuredToolResultStatus.ERROR, - error=f"Failed to connect to Confluence at {self._toolset.confluence_config.url}: {str(e)}", + error=f"Failed to connect to Confluence at {self._toolset.confluence_config.url}: {e!s} | request_context={request_context}", data={"request_context": request_context, "connection_error": str(e)}, - params=params, + params=params_payload, ) @@ return StructuredToolResult( status=StructuredToolResultStatus.ERROR, - error=f"Unexpected error querying Confluence endpoint '{endpoint}': {type(e).__name__}: {str(e)}", + error=f"Unexpected error querying Confluence endpoint '{endpoint}': {type(e).__name__}: {e!s} | request_context={request_context}", data={ "request_context": request_context, "exception_type": type(e).__name__, }, - params=params, + params=params_payload, )
| def _discover_cloud_id(self) -> Optional[str]: | ||
| """Discover cloud_id from Atlassian Cloud using the tenant_info endpoint. | ||
|
|
||
| The /_edge/tenant_info endpoint is a public endpoint that returns the cloud_id | ||
| without requiring authentication, which works for both personal tokens and | ||
| service account scoped tokens. | ||
| """ | ||
| try: | ||
| # Use the public tenant_info endpoint - no auth required | ||
| url = f"{self.confluence_config.url.rstrip('/')}/_edge/tenant_info" | ||
| response = requests.get( | ||
| url, | ||
| headers={"Accept": "application/json"}, | ||
| timeout=self.confluence_config.timeout, | ||
| verify=self.confluence_config.verify_ssl, | ||
| ) | ||
| response.raise_for_status() | ||
| data = response.json() | ||
|
|
||
| cloud_id = data.get("cloudId") | ||
| if cloud_id: | ||
| return cloud_id | ||
| except Exception as e: | ||
| logging.debug(f"Could not auto-discover cloud_id: {e}") | ||
| return None |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the target file
fd "confluence.py" --type fRepository: HolmesGPT/holmesgpt
Length of output: 111
🏁 Script executed:
# Check the Ruff configuration
cat pyproject.toml | head -100Repository: HolmesGPT/holmesgpt
Length of output: 2389
🏁 Script executed:
# Get more details about the confluence.py file and its full context
wc -l holmes/plugins/toolsets/confluence/confluence.pyRepository: HolmesGPT/holmesgpt
Length of output: 115
🏁 Script executed:
# Read the target lines and surrounding context
sed -n '90,130p' holmes/plugins/toolsets/confluence/confluence.pyRepository: HolmesGPT/holmesgpt
Length of output: 1854
🏁 Script executed:
# Check for Ruff configuration in pyproject.toml
grep -A 50 "\[tool.ruff" pyproject.tomlRepository: HolmesGPT/holmesgpt
Length of output: 45
🏁 Script executed:
# Search for other exception handling patterns in the codebase to understand conventions
rg "except.*RequestException" holmes/plugins/toolsets --type py -A 2 | head -50Repository: HolmesGPT/holmesgpt
Length of output: 3952
🏁 Script executed:
# Check what Ruff BLE001 actually is and verify it's a real rule
python3 << 'EOF'
# BLE001 is from flake8-blind-except, now part of Ruff
# It flags catching bare Exception
print("BLE001: Do not catch blind exception: Exception")
print("This is a real Ruff rule for flagging overly broad exception handling")
EOFRepository: HolmesGPT/holmesgpt
Length of output: 149
🏁 Script executed:
# Examine the full confluence.py to understand error handling patterns
cat holmes/plugins/toolsets/confluence/confluence.py | head -50Repository: HolmesGPT/holmesgpt
Length of output: 1393
🏁 Script executed:
# Check what json.JSONDecodeError is and whether it's a ValueError
python3 << 'EOF'
import json
import requests
# Check exception hierarchy
print("json.JSONDecodeError bases:", json.JSONDecodeError.__bases__)
print("Is JSONDecodeError a ValueError?", issubclass(json.JSONDecodeError, ValueError))
# Check what requests.get() can raise
print("\nrequests.RequestException is base for:")
print("- ConnectionError")
print("- HTTPError")
print("- URLRequired")
print("- TooManyRedirects")
print("- ConnectTimeout")
print("- ReadTimeout")
print("- Timeout")
# response.json() raises json.JSONDecodeError which IS a ValueError
print("\nresponse.json() raises: json.JSONDecodeError (subclass of ValueError)")
EOFRepository: HolmesGPT/holmesgpt
Length of output: 149
🏁 Script executed:
# Check the full error handling in this file to see if there are other patterns
rg "except|logging\." holmes/plugins/toolsets/confluence/confluence.py -B 1 -A 3Repository: HolmesGPT/holmesgpt
Length of output: 2902
🏁 Script executed:
# Check what the toolset guidelines say about error context - search for "detailed error message" pattern
rg "detailed error" holmes/plugins/toolsets -A 3 | head -30Repository: HolmesGPT/holmesgpt
Length of output: 434
🏁 Script executed:
# Check the imports in confluence.py to verify requests library usage
head -20 holmes/plugins/toolsets/confluence/confluence.pyRepository: HolmesGPT/holmesgpt
Length of output: 505
🏁 Script executed:
# Verify the specific exception types that would be raised
python3 << 'EOF'
import json
# Verify exception hierarchy for response.json()
try:
# This is what response.json() does internally
json.loads("invalid json")
except Exception as e:
print(f"Exception type from json.loads: {type(e).__name__}")
print(f"Is it a ValueError? {isinstance(e, ValueError)}")
print(f"MRO: {type(e).__mro__}")
EOFRepository: HolmesGPT/holmesgpt
Length of output: 267
🏁 Script executed:
# Check what exceptions the requests library actually raises
python3 << 'EOF'
from requests import RequestException
from requests.exceptions import HTTPError, ConnectionError, Timeout
print("RequestException hierarchy:")
print(f"HTTPError is subclass of RequestException: {issubclass(HTTPError, RequestException)}")
print(f"ConnectionError is subclass of RequestException: {issubclass(ConnectionError, RequestException)}")
print(f"Timeout is subclass of RequestException: {issubclass(Timeout, RequestException)}")
EOFRepository: HolmesGPT/holmesgpt
Length of output: 238
Narrow cloud_id discovery exception handling (Ruff BLE001).
Catching bare Exception violates Ruff BLE001 and can mask unexpected errors. Catch the specific exceptions that requests.get() and response.json() actually raise: requests.RequestException (for HTTP errors) and ValueError (for JSON parsing failures). Also use structured logging format.
🔧 Proposed fix
- except Exception as e:
- logging.debug(f"Could not auto-discover cloud_id: {e}")
+ except (requests.RequestException, ValueError) as e:
+ logging.debug("Could not auto-discover cloud_id: %s", e)🧰 Tools
🪛 Ruff (0.14.13)
120-120: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In `@holmes/plugins/toolsets/confluence/confluence.py` around lines 98 - 122, In
_discover_cloud_id, replace the broad "except Exception" with explicit exception
handling for requests.RequestException (for network/HTTP issues) and ValueError
(for response.json() parse errors), and update the logging call to use
structured logging (e.g., logging.debug("Could not auto-discover cloud_id",
exc_info=e) or logging.debug("Could not auto-discover cloud_id: %s", e)) so
errors aren't masked and are logged consistently; keep the rest of the
request/response logic and return None on handled failures.
| # Include full error context: status code, response text, and parsed JSON if available | ||
| status_code = e.response.status_code | ||
| response_text = e.response.text | ||
| error_json = None | ||
| try: | ||
| error_json = e.response.json() | ||
| except Exception: | ||
| pass | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file structure and the specific lines mentioned
head -n 310 holmes/plugins/toolsets/confluence/confluence.py | tail -n 50Repository: HolmesGPT/holmesgpt
Length of output: 1828
🏁 Script executed:
# Check if logging is imported in the file
grep -n "import logging" holmes/plugins/toolsets/confluence/confluence.pyRepository: HolmesGPT/holmesgpt
Length of output: 79
🏁 Script executed:
# Check the full context around lines 295-303
sed -n '280,310p' holmes/plugins/toolsets/confluence/confluence.pyRepository: HolmesGPT/holmesgpt
Length of output: 1217
🏁 Script executed:
# Check pyproject.toml for Ruff configuration
cat pyproject.toml | grep -A 50 "\[tool.ruff"Repository: HolmesGPT/holmesgpt
Length of output: 45
🏁 Script executed:
# Check if pyproject.toml exists and view it
cat pyproject.toml 2>/dev/null | head -n 200Repository: HolmesGPT/holmesgpt
Length of output: 6355
🏁 Script executed:
# Also check for ruff.toml
cat ruff.toml 2>/dev/null || echo "No ruff.toml found"Repository: HolmesGPT/holmesgpt
Length of output: 969
🏁 Script executed:
# Let's look for Ruff configuration more broadly
find . -name "pyproject.toml" -o -name "ruff.toml" | head -5Repository: HolmesGPT/holmesgpt
Length of output: 91
🏁 Script executed:
# Check if pre-commit hooks are configured
cat .pre-commit-config.yaml 2>/dev/null | grep -A 10 ruff || echo "No pre-commit ruff config found"Repository: HolmesGPT/holmesgpt
Length of output: 493
🏁 Script executed:
# Let's verify the requests library's json() method behavior
python3 << 'EOF'
import json
# Check what exceptions json() can raise
from requests import Response
# Create a sample response with invalid JSON
response_obj = Response()
response_obj._content = b'not json'
response_obj.status_code = 200
try:
response_obj.json()
except Exception as e:
print(f"Exception type: {type(e).__name__}")
print(f"Is it a ValueError? {isinstance(e, ValueError)}")
print(f"Exception module: {type(e).__module__}")
EOFRepository: HolmesGPT/holmesgpt
Length of output: 149
🏁 Script executed:
# Check if logging is used elsewhere in the confluence.py file for error logging
grep -n "logging\." holmes/plugins/toolsets/confluence/confluence.py | head -20Repository: HolmesGPT/holmesgpt
Length of output: 222
🏁 Script executed:
# Verify that the BLE (blind-except) rule is indeed active and not ignored
rg "S110|BLE001|BLE002" ruff.tomlRepository: HolmesGPT/holmesgpt
Length of output: 45
🏁 Script executed:
# Check what the BLE rule actually covers in flake8-bandit/ruff
cat ruff.toml | grep -B5 -A5 "BLE"Repository: HolmesGPT/holmesgpt
Length of output: 424
🏁 Script executed:
# Let's check the requests library documentation for json() method exceptions
# by looking at how it's used elsewhere in the codebase
rg "\.json\(\)" holmes/plugins/toolsets/ -A2 -B2 | head -50Repository: HolmesGPT/holmesgpt
Length of output: 3289
🏁 Script executed:
# Verify the exact Ruff rule code S110 vs BLE001
rg "except.*:" holmes/plugins/toolsets/confluence/confluence.py | head -10Repository: HolmesGPT/holmesgpt
Length of output: 495
🏁 Script executed:
# Verify which Ruff rule code actually corresponds to bare except clauses
rg "BLE001|S110" . --type py | head -5Repository: HolmesGPT/holmesgpt
Length of output: 166
🏁 Script executed:
# Check the requests library behavior - when does json() raise ValueError vs other exceptions
python3 << 'EOF'
# Looking at requests source, response.json() calls json.loads()
# which raises json.JSONDecodeError (subclass of ValueError)
print("JSONDecodeError is a ValueError subclass: True")
print("Rule BLE001: catches bare except / except Exception")
print("Rule S110: is not related to bare except clauses")
EOFRepository: HolmesGPT/holmesgpt
Length of output: 210
🏁 Script executed:
# Verify the actual line numbers and context one more time to ensure accuracy
sed -n '295,305p' holmes/plugins/toolsets/confluence/confluence.pyRepository: HolmesGPT/holmesgpt
Length of output: 394
Narrow exception type and log parse failures (Ruff BLE001).
Bare except Exception: pass violates the blind-except rule (BLE001, not S110) and hides diagnostics needed for error recovery. Per coding guidelines, toolsets must return detailed error context. Narrow to ValueError (json.JSONDecodeError is a ValueError subclass) and log the failure.
🔧 Proposed fix
- try:
- error_json = e.response.json()
- except Exception:
- pass
+ try:
+ error_json = e.response.json()
+ except ValueError as exc:
+ logging.debug("Failed to parse Confluence error JSON: %s", exc)🧰 Tools
🪛 Ruff (0.14.13)
301-302: try-except-pass detected, consider logging the exception
(S110)
301-301: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In `@holmes/plugins/toolsets/confluence/confluence.py` around lines 295 - 303, The
try/except around parsing e.response.json() uses a bare except which hides
errors; change it to catch ValueError (or json.JSONDecodeError) specifically and
log the parse failure instead of silently passing—update the block where
error_json is set (variable error_json and the call e.response.json()) to use
except ValueError as parse_err: logger.debug/exception("Failed to parse error
response JSON", exc_info=parse_err) (or the module's configured logger) so
callers still get detailed error context.
- Add api.atlassian.com to HTTP passthru list in conftest.py (required for Atlassian Cloud API gateway calls) - Fix shell script in test_case.yaml to properly handle Bearer auth for service accounts (previously used broken variable escaping) - Update toolsets.yaml to support service accounts by not requiring username (omitting username triggers Bearer auth) Signed-off-by: Claude <noreply@anthropic.com>
|
/eval |
|
@aantn Your eval run has finished. ✅ Completed successfully 🧪 Manual Eval Results
Results of HolmesGPT evals
Time/Cost columns show % change vs historical average (↑slower/costlier, ↓faster/cheaper). Changes under 10% shown as ±0%. Historical Comparison DetailsFilter: excluding branch 'master' Status: Success - 10 test/model combinations loaded Experiments compared (30):
Comparison indicators:
📖 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: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/llm/fixtures/test_ask_holmes/195_confluence_page_fetch/test_case.yaml`:
- Around line 1-13: The test fixture is missing the required top-level runbooks
field; add an explicit runbooks: {} entry to the YAML in the test case so the
eval schema is satisfied (i.e., update the
test_ask_holmes/195_confluence_page_fetch/test_case.yaml to include runbooks: {}
at the top level alongside user_prompt, expected_output, tags, and
setup_timeout).
♻️ Duplicate comments (3)
holmes/plugins/toolsets/confluence/confluence.py (3)
74-91: Replace broadExceptioncatches in prerequisite & health checks.These blocks still violate Ruff BLE001/RUF010 and mask unexpected failures. Narrow the exception types and use explicit conversion flags.
🔧 Proposed fix
- except Exception as e: - return False, f"Failed to validate Confluence configuration: {str(e)}" + except (ValueError, TypeError, requests.RequestException) as e: + return False, f"Failed to validate Confluence configuration: {e!s}" @@ - except Exception as e: - return False, f"Confluence health check failed: {str(e)}" + except (requests.RequestException, ValueError) as e: + return False, f"Confluence health check failed: {e!s}"Also applies to: 119-150
100-116: Narrow_discover_cloud_idexception handling and avoid f-stringstr(e).Catching
Exceptionhere is flagged by Ruff and hides request/JSON errors.🔧 Proposed fix
- except Exception as e: - logging.debug(f"Could not auto-discover cloud_id: {e}") + except (requests.RequestException, ValueError) as e: + logging.debug("Could not auto-discover cloud_id: %s", e)
289-337: Avoid silent JSON parse failures and broad exception catch in tool errors.The bare
exceptandpasshide parse failures and still trigger Ruff BLE001/S110/RUF010.🔧 Proposed fix
- except Exception: - pass + except ValueError as exc: + logging.debug("Failed to parse Confluence error JSON: %s", exc) @@ - except requests.exceptions.ConnectionError as e: + except requests.exceptions.ConnectionError as e: return StructuredToolResult( status=StructuredToolResultStatus.ERROR, - error=f"Failed to connect to Confluence at {self._toolset.confluence_config.url}: {str(e)}", - data={"request_context": request_context, "connection_error": str(e)}, + error=f"Failed to connect to Confluence at {self._toolset.confluence_config.url}: {e!s}", + data={"request_context": request_context, "connection_error": f"{e!s}"}, params=params, ) - except Exception as e: + except (requests.RequestException, ValueError) as e: return StructuredToolResult( status=StructuredToolResultStatus.ERROR, - error=f"Unexpected error querying Confluence endpoint '{endpoint}': {type(e).__name__}: {str(e)}", + error=f"Unexpected error querying Confluence endpoint '{endpoint}': {type(e).__name__}: {e!s}", data={ "request_context": request_context, "exception_type": type(e).__name__, }, params=params, )
| user_prompt: | | ||
| Find and fetch the contents of the test page titled "HolmesGPT Eval Test Page" in Confluence. | ||
| Tell me the verification code found in the page content. | ||
|
|
||
| expected_output: | ||
| - "Must report the verification code: HOLMES-EVAL-7x9k2m4p" | ||
|
|
||
| tags: | ||
| - confluence | ||
| - question-answer | ||
| - medium | ||
|
|
||
| setup_timeout: 120 |
There was a problem hiding this comment.
Add the required runbooks field to the eval test case.
This fixture should include runbooks: {} even when empty to satisfy the eval schema. As per coding guidelines, add an explicit runbooks entry.
🔧 Proposed fix
tags:
- confluence
- question-answer
- medium
+runbooks: {}
setup_timeout: 120📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| user_prompt: | | |
| Find and fetch the contents of the test page titled "HolmesGPT Eval Test Page" in Confluence. | |
| Tell me the verification code found in the page content. | |
| expected_output: | |
| - "Must report the verification code: HOLMES-EVAL-7x9k2m4p" | |
| tags: | |
| - confluence | |
| - question-answer | |
| - medium | |
| setup_timeout: 120 | |
| user_prompt: | | |
| Find and fetch the contents of the test page titled "HolmesGPT Eval Test Page" in Confluence. | |
| Tell me the verification code found in the page content. | |
| expected_output: | |
| - "Must report the verification code: HOLMES-EVAL-7x9k2m4p" | |
| tags: | |
| - confluence | |
| - question-answer | |
| - medium | |
| runbooks: {} | |
| setup_timeout: 120 |
🤖 Prompt for AI Agents
In `@tests/llm/fixtures/test_ask_holmes/195_confluence_page_fetch/test_case.yaml`
around lines 1 - 13, The test fixture is missing the required top-level runbooks
field; add an explicit runbooks: {} entry to the YAML in the test case so the
eval schema is satisfied (i.e., update the
test_ask_holmes/195_confluence_page_fetch/test_case.yaml to include runbooks: {}
at the top level alongside user_prompt, expected_output, tags, and
setup_timeout).
Simplifies CI/CD setup by reducing required env vars from 3 to 2: - CONFLUENCE_BASE_URL - CONFLUENCE_API_KEY The space key 'MFS' is now hardcoded since we use a dedicated test instance. Signed-off-by: Claude <noreply@anthropic.com>
|
/eval |
|
@aantn Your eval run has finished. ✅ Completed successfully 🧪 Manual Eval Results
Results of HolmesGPT evals
Time/Cost columns show % change vs historical average (↑slower/costlier, ↓faster/cheaper). Changes under 10% shown as ±0%. Historical Comparison DetailsFilter: excluding branch 'master' Status: Success - 9 test/model combinations loaded Experiments compared (30):
Comparison indicators:
📖 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: |
Atlassian Cloud uses /wiki/rest/api/... while self-hosted Confluence (Server/Data Center) uses /rest/api/... without the /wiki prefix. Changes: - Add _get_api_path_prefix() to determine correct API path - Update all tools to use dynamic API prefix - Update docstring with self-hosted configuration example Signed-off-by: Claude <noreply@anthropic.com>
- Remove redundant test 195_confluence_page_fetch (covered by 208) - Update tests 208, 209, 210 toolsets.yaml with Python toolset config (url and api_key instead of relying on env vars directly) Signed-off-by: Claude <noreply@anthropic.com>
This branch has not been deployed
2 participants
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.