From c6aa448ee8eef6860f6397b16d86a98c7cf556b9 Mon Sep 17 00:00:00 2001 From: codex Date: Thu, 6 Aug 2026 15:42:38 -0300 Subject: [PATCH] feat(hermes): add undefined-name and failing-assertion fix categories Categories two and three of the mechanical-fix work, sharing the seam category one established. Undefined or misspelled names are located exactly by the runtime or compiler, so the fix is one identifier or one import. Python names the file on a traceback frame above the error, so the most recent frame is carried forward and attached when the error arrives; a defect with no known file is dropped because the patcher could not act on it. Failing assertions now come from the structured test results junit publishes, carrying the test, its class and the assertion. They deliberately carry no path: the failing test is the symptom and the defect is usually in the code under test, so naming the test file would invite weakening the assertion instead of fixing the cause. The instruction says so explicitly. Co-Authored-By: Claude Opus 5 --- ariadne/services/hermes_code_defects.py | 161 ++++++++++++++++++++++-- ariadne/services/hermes_code_prompt.py | 2 +- tests/test_hermes_code_defects.py | 151 ++++++++++++++++++++++ 3 files changed, 303 insertions(+), 11 deletions(-) diff --git a/ariadne/services/hermes_code_defects.py b/ariadne/services/hermes_code_defects.py index f8129ef..d52896f 100644 --- a/ariadne/services/hermes_code_defects.py +++ b/ariadne/services/hermes_code_defects.py @@ -18,6 +18,8 @@ from typing import Any LINT_VIOLATION = "lint_violation" +UNDEFINED_NAME = "undefined_name" +FAILING_ASSERTION = "failing_assertion" # Each pattern must capture path, line, rule and message. Anything that cannot # name all four is not actionable enough to narrow the patch instruction, so it @@ -47,6 +49,21 @@ _PATTERNS: tuple[tuple[str, re.Pattern[str]], ...] = ( ) _ESLINT_FILE = re.compile(r"^(?P[\w./\-]+\.(?:js|jsx|ts|tsx))$") + +# A missing or misspelled name is named exactly by the runtime or compiler, so +# the fix is one identifier or one import rather than a judgement call. +_NAME_PATTERNS: tuple[tuple[str, re.Pattern[str]], ...] = ( + ("python", re.compile(r"NameError: name '(?P[^']+)' is not defined")), + ("python", re.compile(r"ModuleNotFoundError: No module named '(?P[^']+)'")), + ( + "python", + re.compile(r"ImportError: cannot import name '(?P[^']+)' from '(?P[^']+)'"), + ), + ("go", re.compile(r"^(?P[\w./\-]+\.go):(?P\d+):\d+:\s+undefined:\s+(?P\w+)")), + ("rust", re.compile(r"cannot find (?:value|function|type) `(?P[^`]+)` in this scope")), +) +# The Python traceback names the file on the line above the error itself. +_PY_FRAME = re.compile(r'^\s*File "(?P[^"]+)", line (?P\d+)') _MAX_DEFECTS = 20 @@ -122,6 +139,98 @@ def _scan(lines: list[str], cfg: dict) -> list[dict[str, Any]]: return found +def extract_name_defects(bundle: dict, cfg: dict) -> list[dict[str, Any]]: + """Return the undefined or misspelled names the build reported. + + Inputs: the evidence bundle and the per-job code cfg. Outputs: at most + twenty {"category", "tool", "symbol", "path", "line", "message"} dicts. + + A Python traceback names the file on a frame line above the error, so the + most recent frame is carried forward and attached to the error when it + arrives. A defect whose file is unknown or outside the write allowlist is + dropped, since the patcher could not act on it. Never raises. + """ + + try: + return _scan_names(_evidence_lines(bundle), cfg) + except Exception: + return [] + + +def extract_assertion_defects(bundle: dict, cfg: dict) -> list[dict[str, Any]]: + """Return the failing tests the build published as structured results. + + Inputs: the evidence bundle, whose `jenkins.failed_tests` is populated when + the build published test results, and the per-job code cfg. Outputs: at + most twenty {"category", "test", "class", "message"} dicts. + + Carries no path on purpose: the failing test names the symptom, and the + defect is usually in the code under test rather than in the test itself. + Naming a path here would point the patcher at the test and invite it to + weaken the assertion instead of fixing the cause. Never raises. + """ + + try: + jenkins = bundle.get("jenkins") if isinstance(bundle.get("jenkins"), dict) else {} + tests = jenkins.get("failed_tests") + found = [] + for test in (tests if isinstance(tests, list) else [])[:_MAX_DEFECTS]: + if not isinstance(test, dict) or not str(test.get("name") or "").strip(): + continue + found.append( + { + "category": FAILING_ASSERTION, + "test": str(test.get("name")).strip(), + "class": str(test.get("className") or "").strip(), + "message": " ".join(str(test.get("errorDetails") or "").split())[:400], + } + ) + return found + except Exception: + return [] + + +def _scan_names(lines: list[str], cfg: dict) -> list[dict[str, Any]]: + """Match undefined-name shapes, carrying the last traceback frame.""" + + found: list[dict[str, Any]] = [] + seen: set[tuple[str, str]] = set() + frame_path, frame_line = "", 0 + for raw in lines: + line = raw.rstrip() + frame = _PY_FRAME.match(line) + if frame: + frame_path, frame_line = frame.group("path"), int(frame.group("line")) + continue + for tool, pattern in _NAME_PATTERNS: + match = pattern.search(line) + if not match: + continue + groups = match.groupdict() + path = groups.get("path") or frame_path + line_no = int(groups["line"]) if groups.get("line") else frame_line + if not path or not _is_writable(path, cfg): + break + key = (path, groups["symbol"]) + if key in seen: + break + seen.add(key) + found.append( + { + "category": UNDEFINED_NAME, + "tool": tool, + "symbol": groups["symbol"], + "path": path, + "line": line_no, + "message": line.strip()[:200], + } + ) + break + if len(found) >= _MAX_DEFECTS: + break + return found + + def _is_writable(path: str, cfg: dict) -> bool: """Report whether a diagnostic's file is one the patcher may write to.""" @@ -132,24 +241,56 @@ def _is_writable(path: str, cfg: dict) -> bool: return bool(prefixes) and any(path.startswith(p) for p in prefixes) +def extract_defects(bundle: dict, cfg: dict) -> list[dict[str, Any]]: + """Return every mechanically-fixable defect the evidence names. + + Inputs: the evidence bundle and the per-job code cfg. Outputs: the lint, + undefined-name and failing-assertion defects together, most precisely + located first, so the instruction leads with the defect whose fix is least + open to interpretation. + """ + + return ( + extract_lint_defects(bundle, cfg) + + extract_name_defects(bundle, cfg) + + extract_assertion_defects(bundle, cfg) + ) + + def defect_instruction(defects: list[dict[str, Any]]) -> str: """Render the defects as a precise instruction for the patch prompt. - Inputs: the defects from `extract_lint_defects`. Outputs: a prompt - fragment naming each one, or "" when there are none so the caller falls - back to the open-ended request. Naming the rule and line is the point: - it converts "repair this build" into a change whose correctness can be - read off the same evidence. + Inputs: the defects from `extract_defects`. Outputs: a prompt fragment + naming each one, or "" when there are none so the caller falls back to the + open-ended request. Naming the rule, symbol or assertion is the point: it + converts "repair this build" into a change whose correctness can be read + off the same evidence. """ if not defects: return "" - lines = [ - "A linter reported these violations; fix exactly these and nothing else:", - ] + lines: list[str] = ["The build reported these specific defects; address exactly these:"] for defect in defects: + lines.append(_defect_line(defect)) + if any(d["category"] == LINT_VIOLATION for d in defects): + lines.append("Do not reformat unrelated lines and do not suppress the rule.") + if any(d["category"] == FAILING_ASSERTION for d in defects): lines.append( - f"- {defect['path']} line {defect['line']}: {defect['rule']} {defect['message']}" + "Fix the code under test so the assertion holds. Do not weaken, skip or delete " + "the test, and do not change its expected values." ) - lines.append("Do not reformat unrelated lines and do not suppress the rule.") return "\n".join(lines) + + +def _defect_line(defect: dict[str, Any]) -> str: + """Render one defect as a single instruction bullet.""" + + category = defect["category"] + if category == LINT_VIOLATION: + return f"- {defect['path']} line {defect['line']}: {defect['rule']} {defect['message']}" + if category == UNDEFINED_NAME: + return ( + f"- {defect['path']} line {defect['line']}: the name {defect['symbol']!r} is not " + f"defined or not importable ({defect['message']})" + ) + return f"- test {defect['class']}.{defect['test']} failed: {defect['message']}" diff --git a/ariadne/services/hermes_code_prompt.py b/ariadne/services/hermes_code_prompt.py index db8c463..58c7d1d 100644 --- a/ariadne/services/hermes_code_prompt.py +++ b/ariadne/services/hermes_code_prompt.py @@ -40,7 +40,7 @@ def build_patch_prompt( compact = json.dumps(bundle, separators=(",", ":"), ensure_ascii=True) listing = "\n".join(f"- {path}" for path in fetched) instruction = hermes_code_defects.defect_instruction( - hermes_code_defects.extract_lint_defects(bundle, code_cfg) + hermes_code_defects.extract_defects(bundle, code_cfg) ) sections = "\n\n".join( _FILE_SECTION_TEMPLATE.format(path=path, contents=contents) diff --git a/tests/test_hermes_code_defects.py b/tests/test_hermes_code_defects.py index 68da59e..7a50071 100644 --- a/tests/test_hermes_code_defects.py +++ b/tests/test_hermes_code_defects.py @@ -120,3 +120,154 @@ def test_the_patch_prompt_is_unchanged_without_diagnostics() -> None: assert "A linter reported these violations" not in prompt assert "MINIMAL source fix" in prompt + + +# --- category 2: undefined or misspelled names --------------------------------- + + +def test_python_nameerror_takes_its_file_from_the_traceback_frame() -> None: + """The error line names the symbol; the frame above names the file.""" + + found = module.extract_name_defects( + _bundle(' File "ariadne/app.py", line 42, in build', "NameError: name 'setttings' is not defined"), + CFG, + ) + assert found[0]["symbol"] == "setttings" + assert (found[0]["path"], found[0]["line"]) == ("ariadne/app.py", 42) + assert found[0]["category"] == module.UNDEFINED_NAME + + +def test_import_errors_are_recognised() -> None: + for text in ( + "ModuleNotFoundError: No module named 'httpx'", + "ImportError: cannot import name 'safe_error' from 'ariadne.utils.errors'", + ): + found = module.extract_name_defects( + _bundle(' File "ariadne/app.py", line 3, in ', text), CFG + ) + assert found, text + assert found[0]["category"] == module.UNDEFINED_NAME + + +def test_go_undefined_names_carry_their_own_location() -> None: + found = module.extract_name_defects(_bundle("src/main.go:12:5: undefined: doThing"), CFG) + assert (found[0]["symbol"], found[0]["path"], found[0]["line"]) == ("doThing", "src/main.go", 12) + + +def test_a_name_defect_without_a_known_file_is_dropped() -> None: + """With no frame there is no file, so the patcher could not act on it.""" + + assert module.extract_name_defects(_bundle("NameError: name 'x' is not defined"), CFG) == [] + + +def test_name_defects_outside_the_write_allowlist_are_dropped() -> None: + found = module.extract_name_defects( + _bundle(' File "vendor/thing.py", line 1, in x', "NameError: name 'y' is not defined"), CFG + ) + assert found == [] + + +# --- category 3: failing assertions -------------------------------------------- + + +def test_failing_tests_come_from_structured_results() -> None: + bundle = { + "jenkins": { + "console_failures": [], + "console_tail": "", + "failed_tests": [ + { + "name": "test_discount", + "className": "tests.test_discount", + "errorDetails": "AssertionError: assert 0.0 == 90.0", + } + ], + } + } + found = module.extract_assertion_defects(bundle, CFG) + assert found[0]["test"] == "test_discount" + assert found[0]["class"] == "tests.test_discount" + assert "assert 0.0 == 90.0" in found[0]["message"] + + +def test_an_assertion_defect_carries_no_path() -> None: + """Naming the test file would invite weakening the test instead of fixing the cause.""" + + bundle = {"jenkins": {"failed_tests": [{"name": "t", "className": "c", "errorDetails": "boom"}]}} + assert "path" not in module.extract_assertion_defects(bundle, CFG)[0] + + +def test_assertion_instruction_forbids_weakening_the_test() -> None: + bundle = {"jenkins": {"failed_tests": [{"name": "t", "className": "c", "errorDetails": "boom"}]}} + text = module.defect_instruction(module.extract_assertion_defects(bundle, CFG)) + assert "Fix the code under test" in text + assert "Do not weaken, skip or delete" in text + + +def test_malformed_failed_tests_are_skipped() -> None: + bundle = {"jenkins": {"failed_tests": ["nope", {}, {"name": " "}, {"name": "ok"}]}} + found = module.extract_assertion_defects(bundle, CFG) + assert [f["test"] for f in found] == ["ok"] + + +def test_extract_defects_combines_every_category() -> None: + bundle = { + "jenkins": { + "console_failures": [ + {"text": "ariadne/app.py:12:5: E501 Line too long\n" + ' File "ariadne/app.py", line 42, in build\n' + "NameError: name 'setttings' is not defined"} + ], + "console_tail": "", + "failed_tests": [{"name": "t", "className": "c", "errorDetails": "boom"}], + } + } + cats = {d["category"] for d in module.extract_defects(bundle, CFG)} + assert cats == {module.LINT_VIOLATION, module.UNDEFINED_NAME, module.FAILING_ASSERTION} + + +def test_name_and_assertion_extraction_never_raise_on_hostile_input() -> None: + for bad in (None, 7, {"jenkins": {"console_failures": "no"}}, {"jenkins": {"failed_tests": 5}}): + assert module.extract_name_defects(bad, CFG) == [] + assert module.extract_assertion_defects(bad, CFG) == [] + assert module.extract_name_defects(_bundle("NameError: name 'x' is not defined"), None) == [] + + +def test_name_extraction_is_bounded() -> None: + lines = [] + for i in range(40): + lines.append(f' File "ariadne/f{i}.py", line {i + 1}, in fn') + lines.append(f"NameError: name 'sym{i}' is not defined") + assert len(module.extract_name_defects(_bundle(*lines), CFG)) == module._MAX_DEFECTS + + +def test_duplicate_name_defects_are_collapsed() -> None: + lines = [' File "ariadne/app.py", line 4, in fn', "NameError: name 'zz' is not defined"] * 3 + assert len(module.extract_name_defects(_bundle(*lines), CFG)) == 1 + + +def test_a_name_defect_with_no_prefixes_configured_is_dropped() -> None: + """An empty allowlist must deny, never permit everything.""" + + empty = {"allowed_path_prefixes": [], "allowed_suffixes": [".py"]} + found = module.extract_name_defects( + _bundle(' File "ariadne/app.py", line 1, in fn', "NameError: name 'q' is not defined"), empty + ) + assert found == [] + + +def test_undefined_name_instruction_names_the_symbol() -> None: + found = module.extract_name_defects( + _bundle(' File "ariadne/app.py", line 42, in build', "NameError: name 'setttings' is not defined"), + CFG, + ) + text = module.defect_instruction(found) + assert "'setttings' is not defined or not importable" in text + assert "ariadne/app.py line 42" in text + + +def test_a_lint_defect_with_a_disallowed_suffix_is_dropped() -> None: + """Suffix and prefix are separate gates; both must hold.""" + + cfg = {"allowed_path_prefixes": ["ariadne/"], "allowed_suffixes": [".go"]} + assert module.extract_lint_defects(_bundle("ariadne/app.py:1:1: E501 Long"), cfg) == []