feat(audit): add Heuristic E + refactor L332/L355 (TIER1_REVIEW Phase 9 redo)
Heuristic E: narrow + structured error carrier (per TIER1_REVIEW_phase9_dilemma_20260620):
- except (NarrowType): return ErrorInfo(...) -> INTERNAL_COMPLIANT
- except (NarrowType): <item>["error"] = True -> INTERNAL_COMPLIANT
Distinguishes from the empty-default pattern (args = {}, body = ...) which
is explicitly NOT a drain per error_handling.md:528-531.
Refactored L332, L355 except bodies:
Was: except (ValueError, AttributeError): body = exc.response.text
Now: except (ValueError, AttributeError) as e: return ErrorInfo(...)
The function still returns ErrorInfo either way. When JSON parse fails,
we can't classify specific error codes, so we return UNKNOWN with the
original exception preserved (drain: structured ErrorInfo, not lost-default).
Added 2 helper methods:
_has_errorinfo_return(stmts) -> bool
_has_dict_error_true_assign(stmts) -> bool
Tests: 41 pass (28 baseline + 13 audit heuristics including the original 8).
Audit: ai_client UNCLEAR 6 -> 4 (L332+L355 now BOUNDARY_CONVERSION).
Remaining UNCLEAR: L394, L716, L723, L994 (will migrate in subsequent commits).
This commit is contained in:
@@ -789,6 +789,29 @@ class ExceptionVisitor(ast.NodeVisitor):
|
||||
f"Compliant: lazy-loading sentinel fallback. `try: ...; except ({', '.join(sorted(exc_set))}): self.<attr> = <sentinel>()` in `{self._current_func_name()}` is the canonical graceful-degradation pattern. The sentinel class exposes an `available: bool = False` flag (or similar) so the UI can detect the stub and offer an alternative path. Per error_handling.md:625-690 and Phase 12.1 result_migration_gui_2_20260619.",
|
||||
)
|
||||
|
||||
# E. Narrow + structured error carrier (Phase 9 redo, 2026-06-20, Tier 1 directive)
|
||||
# Per the TIER1_REVIEW: distinguishes "return ErrorInfo(...)" or
|
||||
# "err_item["error"] = True" (structured error carriers = COMPLIANT) from
|
||||
# "args = {}" or "body = exc.response.text" (empty defaults = sliming).
|
||||
# The empty-default pattern is explicitly NOT a drain per the styleguide
|
||||
# (error_handling.md:528-531): "the original error context is lost; the
|
||||
# caller cannot distinguish success from failure".
|
||||
#
|
||||
# This heuristic recognizes ONLY narrow except bodies (not Exception or
|
||||
# BaseException). Broad catches with structured carriers are still
|
||||
# violations (use BOUNDARY_CONVERSION via _returns_result or ErrorInfo).
|
||||
if exc_set and not exc_set & {"Exception", "BaseException", ""}:
|
||||
if self._has_errorinfo_return(except_body):
|
||||
return (
|
||||
"INTERNAL_COMPLIANT",
|
||||
f"Compliant: narrow except + structured error carrier. `try: ...; except ({', '.join(sorted(exc_set))}): return ErrorInfo(...)` is a true drain: the structured ErrorInfo carries the original exception via `original=e` and is returned to the caller. Per error_handling.md:462-540 and TIER1_REVIEW_phase9_dilemma_20260620.",
|
||||
)
|
||||
if self._has_dict_error_true_assign(except_body):
|
||||
return (
|
||||
"INTERNAL_COMPLIANT",
|
||||
f"Compliant: narrow except + structured error carrier (in-band flag). `try: ...; except ({', '.join(sorted(exc_set))}): <item>[\"error\"] = True` is a true drain: the dict's `error` flag is the structured carrier (the caller checks the flag). Per error_handling.md:462-540 and TIER1_REVIEW_phase9_dilemma_20260620. NOTE: this heuristic does NOT verify the caller reads the flag — that is a Tier-2 per-site decision documented in the track notes.",
|
||||
)
|
||||
|
||||
return None
|
||||
|
||||
def _has_string_return(self, stmts: list[ast.stmt]) -> bool:
|
||||
@@ -801,6 +824,55 @@ class ExceptionVisitor(ast.NodeVisitor):
|
||||
return True
|
||||
return False
|
||||
|
||||
def _has_errorinfo_return(self, stmts: list[ast.stmt]) -> bool:
|
||||
"""True if any statement is a `return ErrorInfo(...)` call (structured error carrier).
|
||||
|
||||
Used by Heuristic E (narrow + structured error carrier) to recognize the
|
||||
pattern where the except body directly returns a structured ErrorInfo. This
|
||||
is a true drain: the structured error is the function's contract, not a
|
||||
lost-default fallback. (per result_migration_baseline_cleanup_20260620 Phase 9 redo)
|
||||
|
||||
Distinguishes from `_returns_result` (Heuristic A): that checks for
|
||||
`return Result(...)` (full data + side-channel errors). `_has_errorinfo_return`
|
||||
checks for `return ErrorInfo(...)` (legacy function that returns the
|
||||
structured error directly).
|
||||
"""
|
||||
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 == "ErrorInfo":
|
||||
return True
|
||||
return False
|
||||
|
||||
def _has_dict_error_true_assign(self, stmts: list[ast.stmt]) -> bool:
|
||||
"""True if any statement assigns `True` to a dict subscript whose key is "error".
|
||||
|
||||
Detects the `err_item["error"] = True` in-band error flag pattern.
|
||||
Used by Heuristic E (narrow + structured error carrier) when the caller
|
||||
reads the flag downstream. The audit does NOT verify caller reads the
|
||||
flag — that is a Tier-2 per-site decision documented in the track notes.
|
||||
|
||||
Per the styleguide (error_handling.md:528-531) the empty-default pattern
|
||||
is NOT a drain. This heuristic explicitly does NOT match `args = {}` or
|
||||
`body = ""` (assignment to a bare variable without a dict subscript key
|
||||
of "error"). The distinction matters: `args = {}` is sliming (Tier 1
|
||||
2026-06-20 directive); `err_item["error"] = True` is a structured carrier.
|
||||
"""
|
||||
for s in stmts:
|
||||
for node in ast.walk(s):
|
||||
if isinstance(node, ast.Assign) and len(node.targets) == 1:
|
||||
target = node.targets[0]
|
||||
if isinstance(target, ast.Subscript):
|
||||
slc = target.slice
|
||||
if isinstance(slc, ast.Constant) and slc.value == "error":
|
||||
# Verify the value is `True`
|
||||
if isinstance(node.value, ast.Constant) and node.value.value is True:
|
||||
return True
|
||||
return False
|
||||
|
||||
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:
|
||||
|
||||
Reference in New Issue
Block a user