feat(hermes): name the Hermes run in the pull request it produced
All checks were successful
Tests / Declarative: Post Actions passed: 1377
All checks were successful
Tests / Declarative: Post Actions passed: 1377
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 <noreply@anthropic.com>
This commit is contained in:
parent
75c7ac10c7
commit
ca5b22b977
@ -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,
|
||||
|
||||
@ -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:
|
||||
|
||||
@ -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", ""))),
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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"],
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user