From ca5b22b977685a98727001d23d314853628bffb3 Mon Sep 17 00:00:00 2001 From: codex Date: Thu, 6 Aug 2026 23:16:47 -0300 Subject: [PATCH] feat(hermes): name the Hermes run in the pull request it produced The pull request said "Proposed by Hermes" and gave a reader nothing to check that against. "Hermes made this decision" is a claim; the run id is the receipt, and without it there is no way to open the run, read the prompt the model was given, or see the tool calls it made. The diagnosis issues have carried the run id from the start - the pull requests, which are the more consequential artifact, did not. The unused build_number parameter on open_pull_request is what the run id replaces. That parameter was dead - SonarQube flags it as python:S1172 on this very repository - so the argument count did not grow. Co-Authored-By: Claude Opus 5 --- ariadne/services/hermes_code_flow.py | 2 +- ariadne/services/hermes_code_repair.py | 56 +++++++++++++++++--------- ariadne/services/hermes_code_repos.py | 2 + ariadne/settings.py | 1 + ariadne/settings_hermes.py | 1 + tests/test_hermes_code_flow.py | 1 + tests/test_hermes_code_repair.py | 51 ++++++++++++++++++++--- 7 files changed, 87 insertions(+), 27 deletions(-) diff --git a/ariadne/services/hermes_code_flow.py b/ariadne/services/hermes_code_flow.py index b45eecb..6962bc5 100644 --- a/ariadne/services/hermes_code_flow.py +++ b/ariadne/services/hermes_code_flow.py @@ -412,7 +412,7 @@ def _publish( pull = hermes_code_repair.open_pull_request( code_cfg, incident.incident_id, - incident.build_number, + proposal.run_id, branch, proposal.patch, proposal.analysis, diff --git a/ariadne/services/hermes_code_repair.py b/ariadne/services/hermes_code_repair.py index 0dbb0d2..70abb45 100644 --- a/ariadne/services/hermes_code_repair.py +++ b/ariadne/services/hermes_code_repair.py @@ -115,13 +115,14 @@ def push_branch( return {"branch": branch, "committed": error is None, "error": error} -def open_pull_request( # noqa: PLR0913 - flow contract mirrors push_branch identity fields - cfg: dict, incident_id: str, build_number: int, branch: str, patch: Any, analysis: str +def open_pull_request( # noqa: PLR0913 - the body needs the full proposal provenance + cfg: dict, incident_id: str, run_id: str, branch: str, patch: Any, analysis: str ) -> dict[str, Any]: """Open the human-review pull request for a pushed repair branch. - Inputs: `cfg` as for `fetch_file`; the incident id, failed build number, - pushed branch name, the validated ProposedPatch, and the model analysis. + Inputs: `cfg` as for `fetch_file` plus optional `hermes_ui_url`; the + incident id; the Hermes run that produced the patch; the pushed branch + name; the validated ProposedPatch; and the model analysis. Outputs: {"pr_number", "url", "error"}; an existing PR reported by a 409 counts as success when the payload identifies it. Never raises and never logs the token. @@ -134,7 +135,7 @@ def open_pull_request( # noqa: PLR0913 - flow contract mirrors push_branch iden "head": branch, "base": _base_branch(cfg), "title": f"fix(hermes): repair {incident_id}", - "body": _pr_body(incident_id, patch, analysis), + "body": _pr_body(incident_id, patch, analysis, run_id, cfg), } try: with httpx.Client(timeout=_timeout(cfg)) as client: @@ -255,22 +256,37 @@ def _pr_result(response: Any) -> dict[str, Any]: return {"pr_number": number, "url": url, "error": None} -def _pr_body(incident_id: str, patch: Any, analysis: str) -> str: - """Render the markdown pull-request body for human review.""" +def _pr_body(incident_id: str, patch: Any, analysis: str, run_id: str, cfg: dict) -> str: + """Render the markdown pull-request body for human review. - return "\n".join( - [ - f"## Hermes repair proposal for incident {incident_id}", - "", - f"**Incident:** {incident_id}", - f"**File:** `{patch.path}`", - f"**Analysis:** {analysis}", - f"**Rationale:** {patch.rationale}", - "", - "Proposed by Hermes; validated and pushed by Ariadne; " - "requires human review — no automatic merge.", - ] - ) + Names the Hermes run that produced the patch. "Proposed by Hermes" is a + claim; the run id is the receipt, and without it a reviewer has no way to + read the prompt the model was given or the tool calls it made. The + diagnosis issues have carried this from the start - the pull requests, + which are the more consequential artifact, did not. + """ + + lines = [ + f"## Hermes repair proposal for incident {incident_id}", + "", + f"**Incident:** {incident_id}", + f"**File:** `{patch.path}`", + f"**Analysis:** {analysis}", + f"**Rationale:** {patch.rationale}", + ] + if run_id: + lines.append(f"**Hermes run:** `{run_id}`") + ui_url = str(cfg.get("hermes_ui_url") or "").rstrip("/") + if ui_url: + lines.append( + f"The prompt this run was given and every tool call it made are at {ui_url}." + ) + lines += [ + "", + "Proposed by Hermes; validated and pushed by Ariadne; " + "requires human review — no automatic merge.", + ] + return "\n".join(lines) def _branch_name(ref: Any) -> str: diff --git a/ariadne/services/hermes_code_repos.py b/ariadne/services/hermes_code_repos.py index 0f9be48..a4cfcbb 100644 --- a/ariadne/services/hermes_code_repos.py +++ b/ariadne/services/hermes_code_repos.py @@ -53,6 +53,8 @@ def build_config(config: Any) -> dict[str, Any]: "timeout_seconds": _GITEA_TIMEOUT_SECONDS, "legacy_job": str(getattr(config, "hermes_code_job", "") or ""), "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 ""), "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/settings.py b/ariadne/settings.py index b338ce5..45411b1 100644 --- a/ariadne/settings.py +++ b/ariadne/settings.py @@ -315,6 +315,7 @@ class Settings: jenkins_workspace_cleanup_cron: str testing_triage_cron: str hermes_autotriage_cron: str + hermes_ui_url: str hermes_sonar_cron: str hermes_sonar_enabled: bool hermes_sonar_url: str diff --git a/ariadne/settings_hermes.py b/ariadne/settings_hermes.py index 294a939..dedd69c 100644 --- a/ariadne/settings_hermes.py +++ b/ariadne/settings_hermes.py @@ -50,6 +50,7 @@ def _hermes_autotriage_config() -> dict[str, Any]: "http://hermes.hermes.svc.cluster.local:8642", ).rstrip("/"), "hermes_api_key": _env("ARIADNE_HERMES_API_KEY", ""), + "hermes_ui_url": _env("ARIADNE_HERMES_UI_URL", ""), "hermes_run_timeout_seconds": _env_float("ARIADNE_HERMES_RUN_TIMEOUT_SECONDS", 420.0), # SonarQube sweep. Disabled until projects are named: the map is # sonar-project=jenkins-job, and the job already carries the repository diff --git a/tests/test_hermes_code_flow.py b/tests/test_hermes_code_flow.py index 454768f..3f61b9e 100644 --- a/tests/test_hermes_code_flow.py +++ b/tests/test_hermes_code_flow.py @@ -36,6 +36,7 @@ def _hermes_cfg() -> dict: def _code_cfg(**overrides) -> dict: # type: ignore[no-untyped-def] base = { + "hermes_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 96bc0f2..09e74a7 100644 --- a/tests/test_hermes_code_repair.py +++ b/tests/test_hermes_code_repair.py @@ -295,7 +295,7 @@ def test_open_pull_request_created(monkeypatch) -> None: monkeypatch, [FakeResponse(201, {"number": 5, "html_url": "https://scm.example/pulls/5"})], ) - result = module.open_pull_request(_cfg(), INCIDENT_ID, 7, BRANCH, _patch(), "root cause analysis") + result = module.open_pull_request(_cfg(), INCIDENT_ID, "run_x", BRANCH, _patch(), "root cause analysis") assert result == {"pr_number": 5, "url": "https://scm.example/pulls/5", "error": None} method, url, kwargs = calls["requests"][0] assert (method, url) == ("POST", "https://scm.example/api/v1/repos/bstein/hermes-code-demo/pulls") @@ -314,31 +314,31 @@ def test_open_pull_request_created(monkeypatch) -> None: def test_open_pull_request_conflict_returns_existing(monkeypatch) -> None: _install_http(monkeypatch, [FakeResponse(409, {"number": 9, "html_url": "https://scm.example/pulls/9"})]) - result = module.open_pull_request(_cfg(), INCIDENT_ID, 7, BRANCH, _patch(), "analysis") + result = module.open_pull_request(_cfg(), INCIDENT_ID, "run_x", BRANCH, _patch(), "analysis") assert result == {"pr_number": 9, "url": "https://scm.example/pulls/9", "error": None} def test_open_pull_request_conflict_without_payload(monkeypatch) -> None: _install_http(monkeypatch, [FakeResponse(409)]) - result = module.open_pull_request(_cfg(), INCIDENT_ID, 7, BRANCH, _patch(), "analysis") + result = module.open_pull_request(_cfg(), INCIDENT_ID, "run_x", BRANCH, _patch(), "analysis") assert result == {"pr_number": None, "url": None, "error": "pull request already exists"} def test_open_pull_request_http_error(monkeypatch) -> None: _install_http(monkeypatch, [FakeResponse(500)]) - result = module.open_pull_request(_cfg(), INCIDENT_ID, 7, BRANCH, _patch(), "analysis") + result = module.open_pull_request(_cfg(), INCIDENT_ID, "run_x", BRANCH, _patch(), "analysis") assert result == {"pr_number": None, "url": None, "error": "pull request http 500"} def test_open_pull_request_created_with_bad_payload(monkeypatch) -> None: _install_http(monkeypatch, [FakeResponse(201, {"number": True})]) - result = module.open_pull_request(_cfg(), INCIDENT_ID, 7, BRANCH, _patch(), "analysis") + result = module.open_pull_request(_cfg(), INCIDENT_ID, "run_x", BRANCH, _patch(), "analysis") assert result == {"pr_number": None, "url": None, "error": None} def test_open_pull_request_request_exception(monkeypatch) -> None: _install_http(monkeypatch, [RuntimeError("down")]) - result = module.open_pull_request(_cfg(), INCIDENT_ID, 7, BRANCH, _patch(), "analysis") + result = module.open_pull_request(_cfg(), INCIDENT_ID, "run_x", BRANCH, _patch(), "analysis") assert result == {"pr_number": None, "url": None, "error": "pull request failed: down"} @@ -347,3 +347,42 @@ def test_open_pull_request_without_base_url(monkeypatch) -> None: result = module.open_pull_request(_cfg(gitea_base_url=" "), INCIDENT_ID, 7, BRANCH, _patch(), "a") assert result == {"pr_number": None, "url": None, "error": "gitea base url is empty"} assert calls["requests"] == [] + + +def test_the_pull_request_names_the_hermes_run(monkeypatch) -> None: + """"Proposed by Hermes" is a claim; the run id is the receipt.""" + + calls = _install_http(monkeypatch, [FakeResponse(201, {"number": 5, "html_url": "u"})]) + module.open_pull_request( + {**_cfg(), "hermes_ui_url": "https://agent.bstein.dev/"}, + INCIDENT_ID, + "run_a5af87af", + BRANCH, + _patch(), + "analysis", + ) + + body = calls["requests"][0][2]["json"]["body"] + assert "**Hermes run:** `run_a5af87af`" in body + assert "https://agent.bstein.dev." in body + assert "every tool call it made" in body + + +def test_the_pull_request_omits_the_link_when_no_ui_is_configured(monkeypatch) -> None: + calls = _install_http(monkeypatch, [FakeResponse(201, {"number": 5, "html_url": "u"})]) + module.open_pull_request(_cfg(), INCIDENT_ID, "run_x", BRANCH, _patch(), "analysis") + + body = calls["requests"][0][2]["json"]["body"] + assert "**Hermes run:** `run_x`" in body + assert "http" not in body.split("**Hermes run:**")[1] + + +def test_the_pull_request_stays_readable_without_a_run_id(monkeypatch) -> None: + """A proposal with no run is still worth opening; it just claims less.""" + + calls = _install_http(monkeypatch, [FakeResponse(201, {"number": 5, "html_url": "u"})]) + module.open_pull_request(_cfg(), INCIDENT_ID, "", BRANCH, _patch(), "analysis") + + body = calls["requests"][0][2]["json"]["body"] + assert "Hermes run" not in body + assert "requires human review" in body