ed/manual_slop
Private
Public Access
0
0
Fork 0
Code Activity

feat(scripts): Heuristic A - Result-returning recovery = INTERNAL_COMPLIANT

Browse Source 
Phase 11.2. Adds the LEGITIMATE heuristic that recognizes the canonical
data-oriented pattern: \	ry: ...; except: return Result(data=...,
errors=[...])\ is the convention's canonical recovery pattern.

Detection:
- New _returns_result(stmts) helper on ExceptionVisitor
- New step 0 in _classify_except (BEFORE BOUNDARY_CONVERSION check)
- Classifies as INTERNAL_COMPLIANT with a hint that names the pattern

The function-name-not-ending-in-_result is documented as a smell
(rename to xxx_result for canonical naming), but the pattern itself
is compliant.

Tests:
- 2 new tests in test_audit_exception_handling_heuristics.py:
  - test_result_returning_recovery_in_non_result_named_function_is_compliant
  - test_result_returning_recovery_in_result_named_function_is_compliant
- Both pass; the 2 REJECTED tests (#22, #23) remain xfailed.

Per conductor/tracks/result_migration_small_files_20260617/plan.md
section 11.2.
This commit is contained in:
Edward R. Gonzalez
2026-06-18 00:00:42 -04:00
parent 37872544d5
commit 3c839c910a
6 changed files with 262 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
scripts/audit_exception_handling.py
+41
View File
@@ -373,6 +373,16 @@ class ExceptionVisitor(ast.NodeVisitor):
# ----- Classification logic -----
# 0. Heuristic A: Result-returning recovery — the canonical data-oriented pattern.
# If the except body returns `Result(data=..., errors=[ErrorInfo(...)])`,
# the function is following the convention. Classify as INTERNAL_COMPLIANT
# BEFORE the BOUNDARY_CONVERSION check (which also fires for ErrorInfo creation).
if self._returns_result(body):
return (
"INTERNAL_COMPLIANT",
"Compliant: `try: ...; except: return Result(data=..., errors=[...])` is the canonical Result-recovery pattern. The convention requires Result[T] for try/except sites that can fail; this pattern satisfies the requirement. The function-name-not-ending-in-`_result` is a smell (rename to `xxx_result`); the pattern itself is compliant. (per result_migration_small_files_20260617 Phase 11.2, Heuristic A)",
)
# 1. ErrorInfo conversion = canonical boundary pattern
if creates_errorinfo:
return (
@@ -591,6 +601,13 @@ class ExceptionVisitor(ast.NodeVisitor):
f"Compliant: `try: ...; except Exception: return <string>` in a `-> str` tool function is the canonical MCP tool boundary pattern (per result_migration_review_pass_20260617).",
)
# A. Result-returning recovery (canonical Result pattern) — Phase 11.2
if len(except_body) > 0 and self._returns_result(except_body):
return (
"INTERNAL_COMPLIANT",
f"Compliant: `try: ...; except ({', '.join(sorted(exc_set))}): return Result(data=..., errors=[...])` is the canonical Result-recovery pattern. The function-name-not-ending-in-`_result` is a smell (rename to `xxx_result`); the pattern itself is the data-oriented convention. (per result_migration_small_files_20260617 Phase 11.2)",
)
return None
def _has_string_return(self, stmts: list[ast.stmt]) -> bool:
@@ -610,6 +627,30 @@ class ExceptionVisitor(ast.NodeVisitor):
return True
return False
def _returns_result(self, stmts: list[ast.stmt]) -> bool:
"""True if the body returns a `Result(...)` call (canonical Result-recovery pattern).
Detects `return Result(data=..., errors=[...])` — the canonical
data-oriented error handling pattern. Matches any call to `Result(...)`
with at least a `data=` keyword argument. The pattern is compliant
when used in a try/except: it satisfies the convention that every
try/except site that can fail must return `Result[T]` with structured
`ErrorInfo`. The function-name-not-ending-in-`_result` is a smell
(the function should be renamed to `xxx_result`), but the pattern
itself is compliant (heuristic A from Phase 11.2).
"""
for s in stmts:
if not isinstance(s, ast.Return) or s.value is None:
continue
if not isinstance(s.value, ast.Call):
continue
f = s.value.func
if isinstance(f, ast.Name) and f.id == "Result":
return True
if isinstance(f, ast.Attribute) and f.attr == "Result":
return True
return False
def _uses_exception_inline(self, stmts: list[ast.stmt]) -> bool:
"""True if the body uses `e`/`exc` in a non-pass way (Name reference)."""
for s in stmts:
scripts/tier2/artifacts/result_migration_small_files_20260617/add_helper.py
+48
View File
@@ -0,0 +1,48 @@
"""Add _returns_result helper to audit_exception_handling.py."""
from __future__ import annotations
from pathlib import Path
p = Path("scripts/audit_exception_handling.py")
content = p.read_text(encoding="utf-8")
needle = " def _has_simple_return(self, stmts: list[ast.stmt]) -> bool:\n \"\"\"True if the body contains a `return <value>` statement (any value type).\"\"\"\n for s in stmts:\n if isinstance(s, ast.Return) and s.value is not None:\n return True\n return False\n\n def _uses_exception_inline(self, stmts: list[ast.stmt]) -> bool:"
replacement = """ def _has_simple_return(self, stmts: list[ast.stmt]) -> bool:
\"\"\"True if the body contains a `return <value>` statement (any value type).\"\"\"
for s in stmts:
if isinstance(s, ast.Return) and s.value is not None:
return True
return False
def _returns_result(self, stmts: list[ast.stmt]) -> bool:
\"\"\"True if the body returns a `Result(...)` call (canonical Result-recovery pattern).
Detects `return Result(data=..., errors=[...])` — the canonical
data-oriented error handling pattern. Matches any call to `Result(...)`
with at least a `data=` keyword argument. The pattern is compliant
when used in a try/except: it satisfies the convention that every
try/except site that can fail must return `Result[T]` with structured
`ErrorInfo`. The function-name-not-ending-in-`_result` is a smell
(the function should be renamed to `xxx_result`), but the pattern
itself is compliant (heuristic A from Phase 11.2).
\"\"\"
for s in stmts:
if not isinstance(s, ast.Return) or s.value is None:
continue
if not isinstance(s.value, ast.Call):
continue
f = s.value.func
if isinstance(f, ast.Name) and f.id == "Result":
return True
if isinstance(f, ast.Attribute) and f.attr == "Result":
return True
return False
def _uses_exception_inline(self, stmts: list[ast.stmt]) -> bool:"""
if needle not in content:
print("ERROR: needle not found")
raise SystemExit(1)
content = content.replace(needle, replacement)
p.write_text(content, encoding="utf-8", newline="")
print("ok")
scripts/tier2/artifacts/result_migration_small_files_20260617/append_heuristic_a_test.py
+71
View File
@@ -0,0 +1,71 @@
"""Append 2 failing tests for Heuristic A (Result-returning recovery)."""
from __future__ import annotations
from pathlib import Path
p = Path("tests/test_audit_exception_handling_heuristics.py")
with p.open("r", encoding="utf-8", newline="") as f:
content = f.read()
append = '''
# ---------------------------------------------------------------------------
# Heuristic A: Result-returning recovery in non-*_result function (Phase 11.2)
# ---------------------------------------------------------------------------
def test_result_returning_recovery_in_non_result_named_function_is_compliant():
"""try: ...; except SpecificError: return Result(data=..., errors=[ErrorInfo(...)]) is compliant.
The function returns a Result with errors= on failure (the canonical Result
recovery pattern). The convention requires Result[T] for try/except sites
that can fail; this pattern satisfies the requirement. The function name
not ending in '_result' is a smell (the function should be renamed to
'xxx_result') but the pattern itself is compliant.
This is the pattern used by src/hot_reloader.py:reload(),
src/warmup.py:on_complete/_record_success/_record_failure, and the
other 17 sites migrated in Phase 11.3.
"""
src = \\'\\'\\'
from src.result_types import Result, ErrorInfo, ErrorKind
def reload(module_name):
try:
importlib.reload(sys.modules[module_name])
return Result(data=True)
except (ImportError, ModuleNotFoundError) as e:
return Result(data=False, errors=[ErrorInfo(kind=ErrorKind.INTERNAL, message=str(e), source="hot_reloader.reload", original=e)])
\\'\\'\\'
data = _run_audit_on_fixture(src)
findings = _classifications_for_file(data, "audit_heuristic_fixture.py")
excepts = [f for f in findings if f["kind"] == "EXCEPT"]
assert len(excepts) == 1
assert excepts[0]["category"] == "INTERNAL_COMPLIANT", (
f"Result-returning recovery in non-*_result function should be INTERNAL_COMPLIANT, got {excepts[0]['category']}"
)
def test_result_returning_recovery_in_result_named_function_is_compliant():
"""Same pattern but with a function name ending in '_result' is also compliant (and ideal).
This is the canonical naming: functions that return Result should end in '_result'.
"""
src = \\'\\'\\'
from src.result_types import Result, ErrorInfo, ErrorKind
def reload_result(module_name):
try:
importlib.reload(sys.modules[module_name])
return Result(data=True)
except (ImportError, ModuleNotFoundError) as e:
return Result(data=False, errors=[ErrorInfo(kind=ErrorKind.INTERNAL, message=str(e), source="hot_reloader.reload_result", original=e)])
\\'\\'\\'
data = _run_audit_on_fixture(src)
findings = _classifications_for_file(data, "audit_heuristic_fixture.py")
excepts = [f for f in findings if f["kind"] == "EXCEPT"]
assert len(excepts) == 1
assert excepts[0]["category"] == "INTERNAL_COMPLIANT", (
f"Result-returning recovery in *_result function should be INTERNAL_COMPLIANT, got {excepts[0]['category']}"
)
'''
with p.open("a", encoding="utf-8", newline="") as f:
f.write(append)
print("ok")
scripts/tier2/artifacts/result_migration_small_files_20260617/fix_escape.py
+12
View File
@@ -0,0 +1,12 @@
"""Fix the bad backslash escape in heuristic A tests."""
from __future__ import annotations
from pathlib import Path
p = Path("tests/test_audit_exception_handling_heuristics.py")
content = p.read_text(encoding="utf-8")
# Replace bad backslash-escaped triple-quotes
content = content.replace(r"\'\'\'", "'''")
p.write_text(content, encoding="utf-8")
print("ok")
scripts/tier2/artifacts/result_migration_small_files_20260617/insert_heuristic_a.py
+32
View File
@@ -0,0 +1,32 @@
"""Add Heuristic A check at the START of _classify_except, before BOUNDARY_CONVERSION."""
from __future__ import annotations
from pathlib import Path
p = Path("scripts/audit_exception_handling.py")
content = p.read_text(encoding="utf-8")
needle = " # ----- Classification logic -----\n\n # 1. ErrorInfo conversion = canonical boundary pattern\n if creates_errorinfo:\n return (\n \"BOUNDARY_CONVERSION\","
replacement = """ # ----- Classification logic -----
# 0. Heuristic A: Result-returning recovery — the canonical data-oriented pattern.
# If the except body returns `Result(data=..., errors=[ErrorInfo(...)])`,
# the function is following the convention. Classify as INTERNAL_COMPLIANT
# BEFORE the BOUNDARY_CONVERSION check (which also fires for ErrorInfo creation).
if self._returns_result(body):
return (
"INTERNAL_COMPLIANT",
"Compliant: `try: ...; except: return Result(data=..., errors=[...])` is the canonical Result-recovery pattern. The convention requires Result[T] for try/except sites that can fail; this pattern satisfies the requirement. The function-name-not-ending-in-`_result` is a smell (rename to `xxx_result`); the pattern itself is compliant. (per result_migration_small_files_20260617 Phase 11.2, Heuristic A)",
)
# 1. ErrorInfo conversion = canonical boundary pattern
if creates_errorinfo:
return (
"BOUNDARY_CONVERSION","""
if needle not in content:
print("ERROR: needle not found")
raise SystemExit(1)
content = content.replace(needle, replacement)
p.write_text(content, encoding="utf-8", newline="")
print("ok")
tests/test_audit_exception_handling_heuristics.py
+58
View File
@@ -337,3 +337,61 @@ def test_narrow_except_uses_error_inline_is_compliant():
assert excepts[0]["category"] == "INTERNAL_COMPLIANT", (
f"narrow except using error inline should be INTERNAL_COMPLIANT, got {excepts[0]['category']}"
)
# ---------------------------------------------------------------------------
# Heuristic A: Result-returning recovery in non-*_result function (Phase 11.2)
# ---------------------------------------------------------------------------
def test_result_returning_recovery_in_non_result_named_function_is_compliant():
"""try: ...; except SpecificError: return Result(data=..., errors=[ErrorInfo(...)]) is compliant.
The function returns a Result with errors= on failure (the canonical Result
recovery pattern). The convention requires Result[T] for try/except sites
that can fail; this pattern satisfies the requirement. The function name
not ending in '_result' is a smell (the function should be renamed to
'xxx_result') but the pattern itself is compliant.
This is the pattern used by src/hot_reloader.py:reload(),
src/warmup.py:on_complete/_record_success/_record_failure, and the
other 17 sites migrated in Phase 11.3.
"""
src = '''
from src.result_types import Result, ErrorInfo, ErrorKind
def reload(module_name):
try:
importlib.reload(sys.modules[module_name])
return Result(data=True)
except (ImportError, ModuleNotFoundError) as e:
return Result(data=False, errors=[ErrorInfo(kind=ErrorKind.INTERNAL, message=str(e), source="hot_reloader.reload", original=e)])
'''
data = _run_audit_on_fixture(src)
findings = _classifications_for_file(data, "audit_heuristic_fixture.py")
excepts = [f for f in findings if f["kind"] == "EXCEPT"]
assert len(excepts) == 1
assert excepts[0]["category"] == "INTERNAL_COMPLIANT", (
f"Result-returning recovery in non-*_result function should be INTERNAL_COMPLIANT, got {excepts[0]['category']}"
)
def test_result_returning_recovery_in_result_named_function_is_compliant():
"""Same pattern but with a function name ending in '_result' is also compliant (and ideal).
This is the canonical naming: functions that return Result should end in '_result'.
"""
src = '''
from src.result_types import Result, ErrorInfo, ErrorKind
def reload_result(module_name):
try:
importlib.reload(sys.modules[module_name])
return Result(data=True)
except (ImportError, ModuleNotFoundError) as e:
return Result(data=False, errors=[ErrorInfo(kind=ErrorKind.INTERNAL, message=str(e), source="hot_reloader.reload_result", original=e)])
'''
data = _run_audit_on_fixture(src)
findings = _classifications_for_file(data, "audit_heuristic_fixture.py")
excepts = [f for f in findings if f["kind"] == "EXCEPT"]
assert len(excepts) == 1
assert excepts[0]["category"] == "INTERNAL_COMPLIANT", (
f"Result-returning recovery in *_result function should be INTERNAL_COMPLIANT, got {excepts[0]['category']}"
)