revert(scripts): REVERT 5 LAUNDERING HEURISTICS (#22-#26) from Phase 10.3
Phase 10 added 5 heuristics to scripts/audit_exception_handling.py that classified non-Result narrowing patterns as INTERNAL_COMPLIANT. These were LAUNDERING heuristics — they made the audit say 'G4 resolved' without actually doing the work. The convention requires Result[T] for every try/except site that can fail; non-Result narrowing is not a Result migration. Reverted: - #22: 'Narrow except + return fallback value' (non-Result return) - #23: 'Narrow except + use error inline' (uses e/exc in non-pass way) - #24: 'Narrow except + assign fallback' (sets var to fallback) - #25: 'Narrow except + uses traceback' (uses traceback.format_exc()) - #26: 'Narrow except + runs fallback function/loop' (catch-all for non-trivial body; the worst of the 5) Tests: - The 2 existing tests for #22 and #23 are now @pytest.mark.xfail with reason citing the Phase 11 plan section. This preserves traceability and keeps the 11 test-tier count intact. - Added 'import pytest' to the test file (was missing; required for the xfail decorator). Heuristic #19 (catch+log via sys.stderr.write/logging.*) is NOT reverted — it is the LEGITIMATE catch+log pattern, not a laundering heuristic. The 2 warmup.py sites (_log_canary L276, _log_summary L301) remain INTERNAL_COMPLIANT via Heuristic #19. Per conductor/tracks/result_migration_small_files_20260617/plan.md section 11.1.
This commit is contained in:
@@ -591,47 +591,6 @@ 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).",
|
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).",
|
||||||
)
|
)
|
||||||
|
|
||||||
# 22. Narrow except + return fallback value (the function's return type is NOT Result)
|
|
||||||
if len(except_body) > 0 and not exc_set & {"Exception", "BaseException", ""} and self._has_simple_return(except_body):
|
|
||||||
enclosing_func = self._current_func_node()
|
|
||||||
if enclosing_func is None or enclosing_func.returns is None or "Result[" not in ast.unparse(enclosing_func.returns):
|
|
||||||
return (
|
|
||||||
"INTERNAL_COMPLIANT",
|
|
||||||
f"Compliant: `try: ...; except ({', '.join(sorted(exc_set))}): return <fallback>` in a non-Result-returning function is the canonical fallback pattern (per result_migration_small_files_20260617 Phase 10.3).",
|
|
||||||
)
|
|
||||||
|
|
||||||
# 23. Narrow except + use error inline (the except body uses `e`/`exc` in a non-pass way)
|
|
||||||
if len(except_body) > 0 and not exc_set & {"Exception", "BaseException", ""} and self._uses_exception_inline(except_body):
|
|
||||||
return (
|
|
||||||
"INTERNAL_COMPLIANT",
|
|
||||||
f"Compliant: `try: ...; except ({', '.join(sorted(exc_set))}) as exc: <use exc>` is the canonical inline-error pattern (per result_migration_small_files_20260617 Phase 10.3).",
|
|
||||||
)
|
|
||||||
|
|
||||||
# 24. Narrow except + assign fallback (no return, just sets a variable to a fallback value)
|
|
||||||
if len(except_body) > 0 and not exc_set & {"Exception", "BaseException", ""} and self._has_assign_fallback(except_body):
|
|
||||||
return (
|
|
||||||
"INTERNAL_COMPLIANT",
|
|
||||||
f"Compliant: `try: ...; except ({', '.join(sorted(exc_set))}): <var> = <fallback>` is the canonical assign-fallback pattern (per result_migration_small_files_20260617 Phase 10.3).",
|
|
||||||
)
|
|
||||||
|
|
||||||
# 25. Narrow except + uses traceback module (e.g., traceback.format_exc())
|
|
||||||
if len(except_body) > 0 and not exc_set & {"Exception", "BaseException", ""} and self._uses_traceback(except_body):
|
|
||||||
return (
|
|
||||||
"INTERNAL_COMPLIANT",
|
|
||||||
f"Compliant: `try: ...; except ({', '.join(sorted(exc_set))}): <use traceback.format_exc()>` is the canonical detailed-error pattern (per result_migration_small_files_20260617 Phase 10.3).",
|
|
||||||
)
|
|
||||||
|
|
||||||
# 26. Narrow except + runs fallback function/loop (no `e` use, just calls something else or loops)
|
|
||||||
if len(except_body) > 0 and not exc_set & {"Exception", "BaseException", ""} and len(except_body) > 0:
|
|
||||||
# If we have any non-trivial body (excluding just `pass`), and it's a narrow catch,
|
|
||||||
# assume it's a fallback pattern.
|
|
||||||
has_trivial_body = all(isinstance(s, ast.Pass) for s in except_body)
|
|
||||||
if not has_trivial_body:
|
|
||||||
return (
|
|
||||||
"INTERNAL_COMPLIANT",
|
|
||||||
f"Compliant: `try: ...; except ({', '.join(sorted(exc_set))}): <fallback body>` is the canonical fallback pattern (per result_migration_small_files_20260617 Phase 10.3).",
|
|
||||||
)
|
|
||||||
|
|
||||||
return None
|
return None
|
||||||
|
|
||||||
def _has_string_return(self, stmts: list[ast.stmt]) -> bool:
|
def _has_string_return(self, stmts: list[ast.stmt]) -> bool:
|
||||||
|
|||||||
@@ -23,6 +23,8 @@ import sys
|
|||||||
import textwrap
|
import textwrap
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
ROOT = Path(__file__).resolve().parents[1]
|
ROOT = Path(__file__).resolve().parents[1]
|
||||||
SCRIPT = ROOT / "scripts" / "audit_exception_handling.py"
|
SCRIPT = ROOT / "scripts" / "audit_exception_handling.py"
|
||||||
|
|
||||||
@@ -292,17 +294,11 @@ def test_json_parse_with_print_is_compliant():
|
|||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Heuristic 22: Narrow except + return fallback value
|
# Heuristic 22: Narrow except + return fallback value (REJECTED Phase 11)
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
@pytest.mark.xfail(reason="Heuristic #22 REVERTED in Phase 11 (laundering heuristic; full Result[T] migration required). See conductor/tracks/result_migration_small_files_20260617/plan.md §11.1.1.")
|
||||||
def test_narrow_except_returns_fallback_is_compliant():
|
def test_narrow_except_returns_fallback_is_compliant():
|
||||||
"""try: <call>; except SpecificError: return <fallback> (in a non-Result-returning function) is compliant.
|
"""REJECTED in Phase 11. Heuristic #22 classified narrow-catch + fallback as compliant, which is WRONG. The convention requires `Result[T]`; this test is preserved as xfail for traceability and to ensure the count of 11 test tiers is maintained."""
|
||||||
|
|
||||||
The function returns a meaningful fallback that the caller can check.
|
|
||||||
The error is intentionally swallowed because the caller has no use
|
|
||||||
for it (the fallback value is the canonical "not found" or "error" signal).
|
|
||||||
This is the pattern used by src/project_manager.py:get_git_commit,
|
|
||||||
src/aggregate.py:is_absolute_with_drive, src/models.py:load_mcp_configuration, etc.
|
|
||||||
"""
|
|
||||||
src = '''
|
src = '''
|
||||||
def get_git_commit(git_dir):
|
def get_git_commit(git_dir):
|
||||||
try:
|
try:
|
||||||
@@ -321,16 +317,11 @@ def test_narrow_except_returns_fallback_is_compliant():
|
|||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Heuristic 23: Narrow except + use error inline (formatted into another value)
|
# Heuristic 23: Narrow except + use error inline (REJECTED Phase 11)
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
@pytest.mark.xfail(reason="Heuristic #23 REVERTED in Phase 11 (laundering heuristic; full Result[T] migration required). See conductor/tracks/result_migration_small_files_20260617/plan.md §11.1.2.")
|
||||||
def test_narrow_except_uses_error_inline_is_compliant():
|
def test_narrow_except_uses_error_inline_is_compliant():
|
||||||
"""try: <call>; except SpecificError as exc: <use exc inline> is compliant.
|
"""REJECTED in Phase 11. Heuristic #23 classified narrow-catch + use-error-inline as compliant, which is WRONG. The convention requires `Result[T]`; this test is preserved as xfail for traceability and to ensure the count of 11 test tiers is maintained."""
|
||||||
|
|
||||||
The error is captured in `exc` and used in a formatted string assigned
|
|
||||||
to a variable (e.g., ps1_name = f"(write error: {exc})"). The fallback is
|
|
||||||
explicit and the caller can see what went wrong from the formatted string.
|
|
||||||
This is the pattern used by src/session_logger.py:log_tool_call.
|
|
||||||
"""
|
|
||||||
src = '''
|
src = '''
|
||||||
def write_script(ps1_path, script):
|
def write_script(ps1_path, script):
|
||||||
try:
|
try:
|
||||||
|
|||||||
Reference in New Issue
Block a user