From ee902354c2ef2e5efdf20ef033ef634cd5cb52cc Mon Sep 17 00:00:00 2001 From: codex Date: Fri, 7 Aug 2026 00:05:07 -0300 Subject: [PATCH] feat(hermes): link a sweep proposal to the finding that caused it 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 --- ariadne/services/hermes_code_repair.py | 8 +++++++ ariadne/services/hermes_code_repos.py | 2 ++ ariadne/services/hermes_sonar_client.py | 28 ++++++++++++++++++++++ ariadne/settings.py | 1 + ariadne/settings_hermes.py | 1 + tests/test_hermes_code_flow.py | 1 + tests/test_hermes_code_repair.py | 32 +++++++++++++++++++++++++ tests/test_hermes_sonar_client.py | 24 +++++++++++++++++++ 8 files changed, 97 insertions(+) diff --git a/ariadne/services/hermes_code_repair.py b/ariadne/services/hermes_code_repair.py index 418c22e..545dc97 100644 --- a/ariadne/services/hermes_code_repair.py +++ b/ariadne/services/hermes_code_repair.py @@ -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}", ] diff --git a/ariadne/services/hermes_code_repos.py b/ariadne/services/hermes_code_repos.py index a4cfcbb..050958c 100644 --- a/ariadne/services/hermes_code_repos.py +++ b/ariadne/services/hermes_code_repos.py @@ -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", ""))), diff --git a/ariadne/services/hermes_sonar_client.py b/ariadne/services/hermes_sonar_client.py index fc0b743..981398e 100644 --- a/ariadne/services/hermes_sonar_client.py +++ b/ariadne/services/hermes_sonar_client.py @@ -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//`, 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.""" diff --git a/ariadne/settings.py b/ariadne/settings.py index 45411b1..05b340a 100644 --- a/ariadne/settings.py +++ b/ariadne/settings.py @@ -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] diff --git a/ariadne/settings_hermes.py b/ariadne/settings_hermes.py index dedd69c..fe5c772 100644 --- a/ariadne/settings_hermes.py +++ b/ariadne/settings_hermes.py @@ -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() diff --git a/tests/test_hermes_code_flow.py b/tests/test_hermes_code_flow.py index 3f61b9e..e4156a6 100644 --- a/tests/test_hermes_code_flow.py +++ b/tests/test_hermes_code_flow.py @@ -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"], diff --git a/tests/test_hermes_code_repair.py b/tests/test_hermes_code_repair.py index b304307..2a662b4 100644 --- a/tests/test_hermes_code_repair.py +++ b/tests/test_hermes_code_repair.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"] diff --git a/tests/test_hermes_sonar_client.py b/tests/test_hermes_sonar_client.py index 4bab7ab..749b925 100644 --- a/tests/test_hermes_sonar_client.py +++ b/tests/test_hermes_sonar_client.py @@ -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", "") == ""