feat(hermes): link a sweep proposal to the finding that caused it
All checks were successful
Tests / Declarative: Post Actions passed: 1387
All checks were successful
Tests / Declarative: Post Actions passed: 1387
The pull request named the incident - sonar/ariadne/AZ2y0FYFKy9i4pkIpNlV - and left the reviewer to find what that key meant. Their first question is what the finding actually said: which rule, how severe, what the offending lines look like in context. An opaque key is not an answer to that. The link is derived from the incident id rather than threaded through the flow. The id already carries the project and the key, so a second path for them to travel would only be a second thing that can disagree with the first. A build-driven repair has no finding, so it gets no line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
fe794b8ffe
commit
ee902354c2
@ -7,6 +7,7 @@ from typing import Any
|
||||
import httpx
|
||||
|
||||
from ..utils.logging import get_logger
|
||||
from . import hermes_sonar_client
|
||||
|
||||
|
||||
logger = get_logger(__name__)
|
||||
@ -286,6 +287,13 @@ def _pr_body(incident_id: str, patch: Any, analysis: str, run_id: str, cfg: dict
|
||||
"",
|
||||
f"**Incident:** {incident_id}",
|
||||
f"**File:** `{patch.path}`",
|
||||
]
|
||||
finding = hermes_sonar_client.issue_url(incident_id, cfg.get("sonar_ui_url"))
|
||||
if finding:
|
||||
# A sweep proposal exists because of one finding; the reviewer's first
|
||||
# question is what it said, and an issue key is not an answer.
|
||||
lines.append(f"**SonarQube finding:** {finding}")
|
||||
lines += [
|
||||
f"**Analysis:** {analysis}",
|
||||
f"**Rationale:** {patch.rationale}",
|
||||
]
|
||||
|
||||
@ -55,6 +55,8 @@ def build_config(config: Any) -> dict[str, Any]:
|
||||
"repos": _parse_repo_map(getattr(config, "hermes_code_repos", "")),
|
||||
# Shown in the pull request so a reviewer can read the run that wrote it.
|
||||
"hermes_ui_url": str(getattr(config, "hermes_ui_url", "") or ""),
|
||||
# Shown in sweep proposals so the finding itself is one click away.
|
||||
"sonar_ui_url": str(getattr(config, "hermes_sonar_ui_url", "") or ""),
|
||||
"job_prefixes": _parse_list_map(getattr(config, "hermes_code_prefixes", "")),
|
||||
"job_suffixes": _parse_list_map(getattr(config, "hermes_code_suffixes", "")),
|
||||
"job_base_branches": dict(_parse_pairs(getattr(config, "hermes_code_base_branches", ""))),
|
||||
|
||||
@ -149,6 +149,34 @@ def component_path(component: Any) -> str:
|
||||
return path.strip() if separator else ""
|
||||
|
||||
|
||||
INCIDENT_PREFIX = "sonar/"
|
||||
# The console opens one finding in place, with the rule, the effort estimate
|
||||
# and the offending lines highlighted.
|
||||
_ISSUE_PATH = "/project/issues?resolved=false&id={project}&open={key}"
|
||||
|
||||
|
||||
def issue_url(incident_id: str, ui_url: str) -> str:
|
||||
"""Build the console link for the finding an incident came from.
|
||||
|
||||
Inputs: an incident id shaped `sonar/<project>/<key>`, and the SonarQube
|
||||
base url. Outputs: the deep link, or "" for any incident that did not come
|
||||
from a sweep - a build-driven repair has no finding to point at.
|
||||
|
||||
Parsed from the incident id rather than threaded through the flow: the id
|
||||
already carries both halves, and inventing a second path for them to travel
|
||||
is a second thing that can disagree with the first.
|
||||
"""
|
||||
|
||||
base = str(ui_url or "").rstrip("/")
|
||||
text = str(incident_id or "")
|
||||
if not base or not text.startswith(INCIDENT_PREFIX):
|
||||
return ""
|
||||
parts = text[len(INCIDENT_PREFIX) :].split("/", 1)
|
||||
if len(parts) != 2 or not parts[0].strip() or not parts[1].strip():
|
||||
return ""
|
||||
return base + _ISSUE_PATH.format(project=parts[0].strip(), key=parts[1].strip())
|
||||
|
||||
|
||||
def _types(cfg: dict) -> tuple[str, ...]:
|
||||
"""Return the finding types this deployment allows, defaulting to all."""
|
||||
|
||||
|
||||
@ -319,6 +319,7 @@ class Settings:
|
||||
hermes_sonar_cron: str
|
||||
hermes_sonar_enabled: bool
|
||||
hermes_sonar_url: str
|
||||
hermes_sonar_ui_url: str
|
||||
hermes_sonar_token: str
|
||||
hermes_sonar_projects: dict[str, str]
|
||||
hermes_sonar_types: list[str]
|
||||
|
||||
@ -60,6 +60,7 @@ def _hermes_autotriage_config() -> dict[str, Any]:
|
||||
"ARIADNE_HERMES_SONAR_URL", "http://sonarqube.quality.svc.cluster.local:9000"
|
||||
).rstrip("/"),
|
||||
"hermes_sonar_token": _env("ARIADNE_HERMES_SONAR_TOKEN", ""),
|
||||
"hermes_sonar_ui_url": _env("ARIADNE_HERMES_SONAR_UI_URL", ""),
|
||||
"hermes_sonar_projects": _pair_map(_env("ARIADNE_HERMES_SONAR_PROJECTS", "")),
|
||||
"hermes_sonar_types": [
|
||||
item.strip()
|
||||
|
||||
@ -37,6 +37,7 @@ def _hermes_cfg() -> dict:
|
||||
def _code_cfg(**overrides) -> dict: # type: ignore[no-untyped-def]
|
||||
base = {
|
||||
"hermes_ui_url": "",
|
||||
"sonar_ui_url": "",
|
||||
"candidate_path": "src/discount.py",
|
||||
"allowed_path_prefixes": ["src/"],
|
||||
"allowed_suffixes": [".py"],
|
||||
|
||||
@ -398,3 +398,35 @@ def test_the_run_link_reopens_the_run_in_the_hermes_console() -> None:
|
||||
)
|
||||
assert module.run_url("", "run_x") == ""
|
||||
assert module.run_url("https://agent.bstein.dev", " ") == ""
|
||||
|
||||
|
||||
def test_a_sweep_proposal_links_to_the_finding_that_caused_it(monkeypatch) -> None:
|
||||
"""The reviewer's first question is what the finding said."""
|
||||
|
||||
calls = _install_http(monkeypatch, [FakeResponse(201, {"number": 6, "html_url": "u"})])
|
||||
module.open_pull_request(
|
||||
{**_cfg(), "sonar_ui_url": "https://quality.bstein.dev"},
|
||||
"sonar/ariadne/AZ2y0FYFKy9i4pkIpNlV",
|
||||
"run_x",
|
||||
BRANCH,
|
||||
_patch(),
|
||||
"analysis",
|
||||
)
|
||||
|
||||
body = calls["requests"][0][2]["json"]["body"]
|
||||
assert "**SonarQube finding:** https://quality.bstein.dev/project/issues" in body
|
||||
assert "open=AZ2y0FYFKy9i4pkIpNlV" in body
|
||||
|
||||
|
||||
def test_a_build_driven_proposal_has_no_finding_line(monkeypatch) -> None:
|
||||
calls = _install_http(monkeypatch, [FakeResponse(201, {"number": 6, "html_url": "u"})])
|
||||
module.open_pull_request(
|
||||
{**_cfg(), "sonar_ui_url": "https://quality.bstein.dev"},
|
||||
INCIDENT_ID,
|
||||
"run_x",
|
||||
BRANCH,
|
||||
_patch(),
|
||||
"analysis",
|
||||
)
|
||||
|
||||
assert "SonarQube finding" not in calls["requests"][0][2]["json"]["body"]
|
||||
|
||||
@ -248,3 +248,27 @@ def test_the_timeout_falls_back_when_unparseable() -> None:
|
||||
)
|
||||
def test_the_component_key_yields_the_repository_path(component, expected) -> None:
|
||||
assert module.component_path(component) == expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("incident", "expected"),
|
||||
[
|
||||
(
|
||||
"sonar/ariadne/AZ2y0FYFKy9i4pkIpNlV",
|
||||
"https://quality.bstein.dev/project/issues"
|
||||
"?resolved=false&id=ariadne&open=AZ2y0FYFKy9i4pkIpNlV",
|
||||
),
|
||||
# A build-driven repair has no finding to point at.
|
||||
("ariadne/408", ""),
|
||||
("sonar/ariadne", ""),
|
||||
("sonar//key", ""),
|
||||
("sonar/proj/ ", ""),
|
||||
("", ""),
|
||||
],
|
||||
)
|
||||
def test_the_finding_link_is_derived_from_the_incident_id(incident, expected) -> None:
|
||||
assert module.issue_url(incident, "https://quality.bstein.dev/") == expected
|
||||
|
||||
|
||||
def test_no_finding_link_without_a_configured_console() -> None:
|
||||
assert module.issue_url("sonar/ariadne/AZ1", "") == ""
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user