Improve GitHub tool error diagnostics

[majamassarini]

Improve error reporting for github authenticated tools.

[qodo-for-packit]

PR Summary by Qodo

Improve GitHub tool error diagnostics

✨ Enhancement 🧪 Tests 🕐 10-20 Minutes

AI Description

• Include GitHub API messages in compare and patch tool failures.
• Add repository and reference guidance for failed comparisons.
• Cover enriched 404 and 422 diagnostics with unit tests.

Diagram

sequenceDiagram
    actor Caller
    participant Tools as GitHub Tools
    participant API as GitHub API
    participant Parser as Error Parser
    Caller->>Tools: Fetch patch or compare
    Tools->>API: Authenticated request
    API-->>Tools: Error JSON
    Tools->>Parser: Extract message
    Parser-->>Tools: GitHub detail
    alt Compare returns 404
        Tools-->>Caller: Error with ref guidance
    else Other API error
        Tools-->>Caller: Error with API message
    end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Centralized GitHub request wrapper

• ➕ Applies consistent diagnostics across pull request, compare, and patch tools
• ➕ Removes repeated response-status and error-detail handling
• ➖ Broadens this focused change into a larger refactor
• ➖ Must accommodate JSON metadata, text patches, and distinct tool-specific guidance

Recommendation: Keep the focused shared error-message parser for this PR because it improves the affected paths with minimal risk. Consider a centralized request wrapper if the same diagnostics are later extended to the pull-request tool or additional GitHub operations.

Files changed (2) +69 / -2

Enhancement (1) +25 / -2

github.pyEnrich GitHub compare and patch failures +25/-2

Enrich GitHub compare and patch failures

• Adds a shared helper that extracts human-readable messages from GitHub JSON error responses. Compare and patch failures include those messages, while compare 404 errors also explain that the repository and both references must exist and be comparable.

ymir/tools/privileged/github.py

Tests (1) +44 / -0

test_github.pyTest diagnostic GitHub API errors +44/-0

Test diagnostic GitHub API errors

• Adds a reusable mocked GitHub API error response. New tests verify that patch failures expose GitHub's message and compare 404 failures include actionable reference guidance.

ymir/tools/privileged/tests/unit/test_github.py

[qodo-for-packit]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Malformed GitHub errors escape unwrapped ✓ Resolved 🐞 Bug ☼ Reliability

Description

_github_error_message awaits response.json(...) but its exception list omits
UnicodeDecodeError, which can be raised while decoding an error body before JSON parsing. When
either changed caller receives a 4xx or non-retried 5xx body with invalid text encoding, that
exception bypasses its outer handler and escapes instead of becoming a ToolError.

Code

ymir/tools/privileged/github.py[R55-57]

+        data = await response.json(content_type=None)
+    except (aiohttp.ClientError, JSONDecodeError, TypeError):
+        return None

Relevance

●●● Strong

Defensive exception handling for malformed responses matches accepted reliability fixes in
privileged tools.

PR-#642
PR-#729

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new helper parses every error response but catches only client, JSON syntax, and type errors;
both callers invoke it before raise_for_status, while their surrounding handlers likewise omit
text-decoding errors.

ymir/tools/privileged/github.py[52-60]
ymir/tools/privileged/github.py[185-207]
ymir/tools/privileged/github.py[258-282]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`_github_error_message` can raise `UnicodeDecodeError` while decoding a malformed HTTP error body, bypassing the normal `ToolError` handling in both GitHub tools.

## Fix Focus Areas
- ymir/tools/privileged/github.py[52-60]
- ymir/tools/privileged/tests/unit/test_github.py[66-98]

## Recommended Fix
Treat response text-decoding and invalid-charset failures like malformed JSON by catching the relevant decoding exceptions in `_github_error_message` and returning `None`. Add a test whose mocked error response raises `UnicodeDecodeError` from `json()` and verify each affected tool still raises its normal `ToolError`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Server outages still hide GitHub details ✓ Resolved 🐞 Bug ≡ Correctness

Description

aiohttp_get_with_retries raises ClientResponseError before yielding the final retryable
response, so the newly added _github_error_message calls cannot inspect its body. When a GitHub
503 persists through all retries, both compare and patch errors omit the available GitHub message
and report only the generic retry exception.

Code

ymir/tools/privileged/github.py[R190-191]

+                if response.status >= 400:
+                    github_error = await _github_error_message(response)

Relevance

●● Moderate

Likely valid diagnostic gap, but fixing requires changing shared retry semantics; historical
evidence is indirect.

PR-#581
PR-#642

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The repository configures 503 as retryable, and the shared helper raises directly on its final
attempt rather than yielding that response. The diagnostic parsing exists only inside the
yielded-response blocks, leaving github_error unset for every exhausted 503.

ymir/tools/constants.py[3-6]
ymir/tools/http.py[19-40]
ymir/tools/privileged/github.py[183-207]
ymir/tools/privileged/github.py[257-282]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The GitHub diagnostic parser is unreachable for an exhausted 503 because `aiohttp_get_with_retries` raises before yielding the final response.

## Fix Focus Areas
- ymir/tools/http.py[19-40]
- ymir/tools/privileged/github.py[185-207]
- ymir/tools/privileged/github.py[259-282]
- ymir/tools/privileged/tests/unit/test_http.py[89-107]

## Recommended Fix
Extend the retry helper with an opt-in mode that yields the final retryable response instead of raising, preserving its existing default for other callers. Enable that mode for the GitHub compare and patch requests so they can parse the body and then call `raise_for_status`, and add an exhausted-503 test asserting that the GitHub message appears.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Compliance rules (platform): 8 rules

Review mode: ⚖️ Balanced: This behavioral change spans shared HTTP retry semantics and two GitHub error-handling paths, creating real but localized runtime risk that warrants a complete single-pass review.

Tip of the day

💡 Did you know, you can choose which labels appear on a finding, and whether they show icons or text

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[majamassarini]

/agentic_review

[qodo-for-packit]

Code review by qodo was updated up to the latest commit 145e121

[lbarcziova]

LGTM