Private
Public Access
Merge remote-tracking branch 'tier2-clone/tier2/result_migration_baseline_cleanup_20260620'
This commit is contained in:
@@ -296,3 +296,93 @@ def test_lazy_loading_sentinel_fallback_in_get_is_compliant():
|
||||
f"(direct `self._cached = _BarStub()`) should be INTERNAL_COMPLIANT "
|
||||
f"(canonical graceful-degradation pattern); got {category}. Hint: {hint}"
|
||||
)
|
||||
|
||||
|
||||
# ============ Phase 9 redo: Heuristic E regression tests (TIER1_REVIEW) ============
|
||||
|
||||
def test_heuristic_e_narrow_return_errorinfo_is_compliant():
|
||||
"""Phase 9 redo: narrow except + return ErrorInfo(...) is a true drain.
|
||||
|
||||
Per TIER1_REVIEW_phase9_dilemma_20260620: a narrow except body that
|
||||
returns a structured ErrorInfo carries the original exception and is
|
||||
the function's contract. This is NOT sliming (the error context is
|
||||
preserved in `original=e`).
|
||||
"""
|
||||
src = (
|
||||
"def _classify_anthropic_error(exc, source):\n"
|
||||
" try:\n"
|
||||
" err_data = exc.response.json()\n"
|
||||
" except (ValueError, AttributeError) as e:\n"
|
||||
" return ErrorInfo(kind=ErrorKind.UNKNOWN, message=str(e), source=source, original=e)\n"
|
||||
)
|
||||
visitor = _make_visitor(src, "_classify_anthropic_error")
|
||||
try_node = _find_handler(visitor)
|
||||
handler = try_node.handlers[0]
|
||||
category, hint = visitor._classify_except(handler, try_node)
|
||||
assert category in ("INTERNAL_COMPLIANT", "BOUNDARY_CONVERSION"), (
|
||||
f"Heuristic E regression: narrow except + return ErrorInfo(...) "
|
||||
f"should be a compliant classification (INTERNAL_COMPLIANT via Heuristic E "
|
||||
f"or BOUNDARY_CONVERSION via existing creates_errorinfo check); got {category}. Hint: {hint}"
|
||||
)
|
||||
|
||||
|
||||
def test_heuristic_e_narrow_dict_error_true_assign_is_compliant():
|
||||
"""Phase 9 redo: narrow except + dict[error] = True is a true drain (in-band flag).
|
||||
|
||||
Per TIER1_REVIEW: `except (NarrowType) as e: item["error"] = True`
|
||||
is a structured error carrier. The caller is expected to inspect the
|
||||
`error` flag (per-site decision documented in track notes; the audit
|
||||
does NOT verify caller reads the flag).
|
||||
"""
|
||||
src = (
|
||||
"def _reread_file_items(file_items):\n"
|
||||
" try:\n"
|
||||
" content = p.read_text()\n"
|
||||
" new_item = {**item, 'content': content}\n"
|
||||
" except (OSError, UnicodeDecodeError) as e:\n"
|
||||
" err_item = {**item, 'content': f'ERROR: {e}'}\n"
|
||||
" err_item['error'] = True\n"
|
||||
" refreshed.append(err_item)\n"
|
||||
)
|
||||
visitor = _make_visitor(src, "_reread_file_items")
|
||||
try_node = _find_handler(visitor)
|
||||
handler = try_node.handlers[0]
|
||||
category, hint = visitor._classify_except(handler, try_node)
|
||||
assert category == "INTERNAL_COMPLIANT", (
|
||||
f"Heuristic E regression: narrow except + dict['error'] = True "
|
||||
f"should be INTERNAL_COMPLIANT (in-band error flag carrier); got {category}. Hint: {hint}"
|
||||
)
|
||||
|
||||
|
||||
def test_heuristic_e_empty_default_args_is_NOT_compliant():
|
||||
"""Phase 9 redo: narrow except + args = {} is NOT a drain (sliming).
|
||||
|
||||
Per TIER1_REVIEW: the empty-default pattern loses error context. The
|
||||
caller cannot distinguish success from failure. Heuristic E
|
||||
explicitly does NOT match this pattern (this test is a regression
|
||||
guard against future "helpful" heuristic additions that would
|
||||
laundering this sliming pattern).
|
||||
|
||||
Structure: extract into a helper function so the try is at the top
|
||||
level of the function body (required by _find_handler test helper).
|
||||
"""
|
||||
src = (
|
||||
"def _parse_tool_args(tool_args_str):\n"
|
||||
" try:\n"
|
||||
" args = json.loads(tool_args_str)\n"
|
||||
" except (ValueError, TypeError):\n"
|
||||
" args = {}\n"
|
||||
" return args\n"
|
||||
)
|
||||
visitor = _make_visitor(src, "_parse_tool_args")
|
||||
try_node = _find_handler(visitor)
|
||||
handler = try_node.handlers[0]
|
||||
category, hint = visitor._classify_except(handler, try_node)
|
||||
# The site is narrow + non-broad but the body is empty-default.
|
||||
# Heuristic E should NOT classify as COMPLIANT. May be INTERNAL_BROAD_CATCH
|
||||
# (no drain) or UNCLEAR. NOT INTERNAL_COMPLIANT or BOUNDARY_CONVERSION.
|
||||
assert category not in ("INTERNAL_COMPLIANT", "BOUNDARY_CONVERSION"), (
|
||||
f"Heuristic E regression: narrow except + args = {{}} (empty default) "
|
||||
f"must NOT be classified as compliant (INTERNAL_COMPLIANT or BOUNDARY_CONVERSION "
|
||||
f"would be sliming per TIER1_REVIEW). Got {category} which would laundering the pattern. Hint: {hint}"
|
||||
)
|
||||
|
||||
@@ -0,0 +1,362 @@
|
||||
"""Invariant tests for result_migration_baseline_cleanup_20260620.
|
||||
|
||||
Phase 1 (4): audit + inventory doc counts match expected baseline
|
||||
Phase 2 (3): baseline state is correct (88 MIG sites in 3 files)
|
||||
Phase 3 (3): mcp_client BC count decreased from 40 -> 32 after Batch A
|
||||
Phase 4 (3): mcp_client BC count decreased from 32 -> 24 after Batch B
|
||||
Phase 5 (3): mcp_client BC count decreased from 24 -> 16 after Batch C
|
||||
Phase 6 (3): mcp_client BC count decreased from 16 -> 9 after Batch D
|
||||
Phase 7 (3): mcp_client BC count decreased from 9 -> <=3 after Batch E
|
||||
"""
|
||||
import json
|
||||
import subprocess
|
||||
from collections import Counter
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
AUDIT_PATH = Path("tests/artifacts/PHASE1_AUDIT_BASELINE.json")
|
||||
INV_MCP = Path("tests/artifacts/PHASE1_INVENTORY_mcp_client.md")
|
||||
INV_AI = Path("tests/artifacts/PHASE1_INVENTORY_ai_client.md")
|
||||
INV_RAG = Path("tests/artifacts/PHASE1_INVENTORY_rag_engine.md")
|
||||
|
||||
MIG = {"INTERNAL_BROAD_CATCH", "INTERNAL_SILENT_SWALLOW", "INTERNAL_OPTIONAL_RETURN", "INTERNAL_RETHROW", "UNCLEAR"}
|
||||
EXPECTED = {
|
||||
"src\\mcp_client.py": (40, 5, 0, 0, 1, 46),
|
||||
"src\\ai_client.py": (17, 9, 0, 7, 0, 33),
|
||||
"src\\rag_engine.py": (5, 1, 0, 3, 0, 9),
|
||||
}
|
||||
TARGETS = ("src\\mcp_client.py", "src\\ai_client.py", "src\\rag_engine.py")
|
||||
|
||||
|
||||
def _load_audit():
|
||||
return json.loads(AUDIT_PATH.read_text(encoding="utf-8"))
|
||||
|
||||
|
||||
def _audit_live():
|
||||
r = subprocess.run(
|
||||
["uv", "run", "python", "scripts/audit_exception_handling.py",
|
||||
"--include-baseline", "--json"],
|
||||
capture_output=True, text=True
|
||||
)
|
||||
return json.loads(r.stdout)
|
||||
|
||||
|
||||
# ============ Phase 1 tests (4) ============
|
||||
|
||||
def test_phase1_audit_json_exists():
|
||||
assert AUDIT_PATH.exists(), f"missing audit json at {AUDIT_PATH}"
|
||||
|
||||
|
||||
def test_phase1_inventory_docs_exist():
|
||||
for p in [INV_MCP, INV_AI, INV_RAG]:
|
||||
assert p.exists(), f"missing inventory doc at {p}"
|
||||
assert p.stat().st_size > 500, f"inventory doc {p} too small"
|
||||
|
||||
|
||||
def test_phase1_total_migration_target_is_88():
|
||||
data = _load_audit()
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
total = 0
|
||||
for key in EXPECTED:
|
||||
findings = files[key]["findings"]
|
||||
mig = [f for f in findings if f["category"] in MIG]
|
||||
total += len(mig)
|
||||
assert total == 88, f"expected 88 migration-target sites, got {total}"
|
||||
|
||||
|
||||
def test_phase1_per_file_site_counts():
|
||||
data = _load_audit()
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
for key, expected in EXPECTED.items():
|
||||
findings = files[key]["findings"]
|
||||
cats = Counter(f["category"] for f in findings)
|
||||
bc = cats.get("INTERNAL_BROAD_CATCH", 0)
|
||||
ss = cats.get("INTERNAL_SILENT_SWALLOW", 0)
|
||||
opt = cats.get("INTERNAL_OPTIONAL_RETURN", 0)
|
||||
rethrow = cats.get("INTERNAL_RETHROW", 0)
|
||||
unclear = cats.get("UNCLEAR", 0)
|
||||
mig = bc + ss + opt + rethrow + unclear
|
||||
assert (bc, ss, opt, rethrow, unclear, mig) == expected, (
|
||||
f"{key}: expected BC={expected[0]} SS={expected[1]} OPT={expected[2]} "
|
||||
f"RETHROW={expected[3]} UNCLEAR={expected[4]} MIG={expected[5]}, "
|
||||
f"got BC={bc} SS={ss} OPT={opt} RETHROW={rethrow} UNCLEAR={unclear} MIG={mig}"
|
||||
)
|
||||
|
||||
|
||||
# ============ Phase 2 tests (3) ============
|
||||
|
||||
def test_phase2_baseline_audit_runs():
|
||||
r = subprocess.run(
|
||||
["uv", "run", "python", "scripts/audit_exception_handling.py",
|
||||
"--include-baseline", "--json"],
|
||||
capture_output=True, text=True
|
||||
)
|
||||
assert r.returncode == 0, f"audit failed: {r.stderr[:500]}"
|
||||
data = json.loads(r.stdout)
|
||||
assert "files" in data
|
||||
assert len(data["files"]) >= 40, f"expected 40+ files, got {len(data['files'])}"
|
||||
|
||||
|
||||
def test_phase2_all_3_targets_have_migration_sites():
|
||||
data = _load_audit()
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
for target in TARGETS:
|
||||
assert target in files, f"missing target file: {target}"
|
||||
mig = [f for f in files[target]["findings"] if f["category"] in MIG]
|
||||
assert len(mig) > 0, f"{target} has 0 migration-target sites (expected >0)"
|
||||
|
||||
|
||||
def test_phase2_per_file_baseline_counts_match_inventory():
|
||||
data = _load_audit()
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
BASELINE = {"src\\mcp_client.py": 46, "src\\ai_client.py": 33, "src\\rag_engine.py": 9}
|
||||
for target, expected in BASELINE.items():
|
||||
mig = [f for f in files[target]["findings"] if f["category"] in MIG]
|
||||
assert len(mig) == expected, (
|
||||
f"{target}: baseline expected {expected}, got {len(mig)}"
|
||||
)
|
||||
|
||||
|
||||
# ============ Phase 3 tests (3) ============
|
||||
|
||||
def test_phase3_mcp_client_broad_catch_decreased_from_40_to_32():
|
||||
"""Loosened: BC <= 32 to allow Phase 4+ overshoot."""
|
||||
data = _audit_live()
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
findings = files["src\\mcp_client.py"]["findings"]
|
||||
bc = sum(1 for f in findings if f["category"] == "INTERNAL_BROAD_CATCH")
|
||||
assert bc <= 32, f"expected mcp_client BC<=32 after Phase 3, got {bc}"
|
||||
|
||||
|
||||
def test_phase3_total_migration_target_decreased_to_80():
|
||||
"""Loosened: total MIG <= 80."""
|
||||
data = _audit_live()
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
total = 0
|
||||
for key in TARGETS:
|
||||
findings = files[key]["findings"]
|
||||
total += sum(1 for f in findings if f["category"] in MIG)
|
||||
assert total <= 80, f"expected total MIG<=80 after Phase 3, got {total}"
|
||||
|
||||
|
||||
def test_phase3_audit_baseline_matches_phase1_audit_json():
|
||||
data = _load_audit()
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
total = 0
|
||||
for key in TARGETS:
|
||||
findings = files[key]["findings"]
|
||||
total += sum(1 for f in findings if f["category"] in MIG)
|
||||
assert total == 88, f"PHASE1_AUDIT_BASELINE.json expected 88 baseline MIG, got {total}"
|
||||
|
||||
|
||||
# ============ Phase 4 tests (3) ============
|
||||
|
||||
def test_phase4_mcp_client_broad_catch_decreased_to_24():
|
||||
"""Loosened: BC <= 24."""
|
||||
data = _audit_live()
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
findings = files["src\\mcp_client.py"]["findings"]
|
||||
bc = sum(1 for f in findings if f["category"] == "INTERNAL_BROAD_CATCH")
|
||||
assert bc <= 24, f"expected mcp_client BC<=24 after Phase 4, got {bc}"
|
||||
|
||||
|
||||
def test_phase4_total_migration_target_decreased_to_72():
|
||||
"""Loosened: total MIG <= 72."""
|
||||
data = _audit_live()
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
total = 0
|
||||
for key in TARGETS:
|
||||
findings = files[key]["findings"]
|
||||
total += sum(1 for f in findings if f["category"] in MIG)
|
||||
assert total <= 72, f"expected total MIG<=72 after Phase 4, got {total}"
|
||||
|
||||
|
||||
def test_phase4_modules_import_cleanly():
|
||||
"""Verify mcp_client module imports after Batch B."""
|
||||
import src.mcp_client
|
||||
assert hasattr(src.mcp_client, "get_git_diff_result")
|
||||
assert hasattr(src.mcp_client, "ts_c_get_skeleton_result")
|
||||
|
||||
|
||||
# ============ Phase 5 tests (3) ============
|
||||
|
||||
def test_phase5_mcp_client_broad_catch_decreased_to_16():
|
||||
"""Loosened: BC <= 16."""
|
||||
data = _audit_live()
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
findings = files["src\\mcp_client.py"]["findings"]
|
||||
bc = sum(1 for f in findings if f["category"] == "INTERNAL_BROAD_CATCH")
|
||||
assert bc <= 16, f"expected mcp_client BC<=16 after Phase 5, got {bc}"
|
||||
|
||||
|
||||
def test_phase5_total_migration_target_decreased_to_64():
|
||||
"""Loosened: total MIG <= 64."""
|
||||
data = _audit_live()
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
total = 0
|
||||
for key in TARGETS:
|
||||
findings = files[key]["findings"]
|
||||
total += sum(1 for f in findings if f["category"] in MIG)
|
||||
assert total <= 64, f"expected total MIG<=64 after Phase 5, got {total}"
|
||||
|
||||
|
||||
def test_phase5_modules_import_cleanly():
|
||||
"""Verify mcp_client module imports after Batch C."""
|
||||
import src.mcp_client
|
||||
assert hasattr(src.mcp_client, "ts_cpp_get_definition_result")
|
||||
assert hasattr(src.mcp_client, "py_get_skeleton_result")
|
||||
assert hasattr(src.mcp_client, "py_get_code_outline_result")
|
||||
|
||||
|
||||
# ============ Phase 6 tests (3) ============
|
||||
|
||||
def test_phase6_mcp_client_broad_catch_decreased_to_9():
|
||||
"""Loosened: BC <= 9."""
|
||||
data = _audit_live()
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
findings = files["src\\mcp_client.py"]["findings"]
|
||||
bc = sum(1 for f in findings if f["category"] == "INTERNAL_BROAD_CATCH")
|
||||
assert bc <= 9, f"expected mcp_client BC<=9 after Phase 6, got {bc}"
|
||||
|
||||
|
||||
def test_phase6_total_migration_target_decreased_to_56():
|
||||
"""Loosened: total MIG <= 56."""
|
||||
data = _audit_live()
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
total = 0
|
||||
for key in TARGETS:
|
||||
findings = files[key]["findings"]
|
||||
total += sum(1 for f in findings if f["category"] in MIG)
|
||||
assert total <= 56, f"expected total MIG<=56 after Phase 6, got {total}"
|
||||
|
||||
|
||||
def test_phase6_modules_import_cleanly():
|
||||
"""Verify mcp_client module imports after Batch D."""
|
||||
import src.mcp_client
|
||||
assert hasattr(src.mcp_client, "py_get_signature_result")
|
||||
assert hasattr(src.mcp_client, "py_set_signature_result")
|
||||
assert hasattr(src.mcp_client, "py_check_syntax_result")
|
||||
|
||||
|
||||
# ============ Phase 7 tests (3) ============
|
||||
|
||||
def test_phase7_mcp_client_broad_catch_decreased():
|
||||
"""After Phase 7 Batch E, mcp_client BC <= 3 (the 3 nested helper functions)."""
|
||||
data = _audit_live()
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
findings = files["src\\mcp_client.py"]["findings"]
|
||||
bc = sum(1 for f in findings if f["category"] == "INTERNAL_BROAD_CATCH")
|
||||
assert bc <= 3, f"expected mcp_client BC<=3 after Phase 7, got {bc}"
|
||||
|
||||
|
||||
def test_phase7_total_migration_target_decreased():
|
||||
"""Total MIG was 56 after Phase 6; should be <= 48 after Phase 7 (8 sites migrated)."""
|
||||
data = _audit_live()
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
total = 0
|
||||
for key in TARGETS:
|
||||
findings = files[key]["findings"]
|
||||
total += sum(1 for f in findings if f["category"] in MIG)
|
||||
assert total <= 48, f"expected total MIG<=48 after Phase 7, got {total}"
|
||||
|
||||
|
||||
def test_phase7_modules_import_cleanly():
|
||||
"""Verify mcp_client module imports after Phase 7 Batch E migrations."""
|
||||
import src.mcp_client
|
||||
assert hasattr(src.mcp_client, "py_get_docstring_result")
|
||||
assert hasattr(src.mcp_client, "derive_code_path_result")
|
||||
assert hasattr(src.mcp_client, "get_tree_result")
|
||||
assert hasattr(src.mcp_client, "web_search_result")
|
||||
assert hasattr(src.mcp_client, "fetch_url_result")
|
||||
assert hasattr(src.mcp_client, "get_ui_performance_result")
|
||||
|
||||
# ============ Phase 8 tests (3) ============
|
||||
|
||||
def test_phase8_mcp_client_silent_swallow_zero():
|
||||
"""Phase 8 CRITICAL anti-sliming phase: mcp_client INTERNAL_SILENT_SWALLOW = 0."""
|
||||
data = _audit_live()
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
findings = files["src\\mcp_client.py"]["findings"]
|
||||
ss = sum(1 for f in findings if f["category"] == "INTERNAL_SILENT_SWALLOW")
|
||||
assert ss == 0, f"expected mcp_client SS=0 after Phase 8, got {ss}"
|
||||
|
||||
|
||||
def test_phase8_mcp_client_total_migration_target_zero():
|
||||
"""After Phase 8, mcp_client should have 0 migration-target sites (BC + SS + UNCLEAR)."""
|
||||
data = _audit_live()
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
findings = files["src\\mcp_client.py"]["findings"]
|
||||
mig_cats = {"INTERNAL_BROAD_CATCH", "INTERNAL_SILENT_SWALLOW", "UNCLEAR"}
|
||||
total = sum(1 for f in findings if f["category"] in mig_cats)
|
||||
assert total == 0, f"expected mcp_client migration-target=0 after Phase 8, got {total}"
|
||||
|
||||
|
||||
def test_phase8_modules_import_cleanly():
|
||||
"""Verify mcp_client imports after Phase 8 anti-sliming migrations."""
|
||||
import src.mcp_client
|
||||
# New _result variants from Phase 8 are inside py_find_usages_result and
|
||||
# derive_code_path_result; these are integration tests, not attribute tests.
|
||||
assert hasattr(src.mcp_client, "py_find_usages_result")
|
||||
assert hasattr(src.mcp_client, "derive_code_path_result")
|
||||
|
||||
|
||||
# ============ Phase 9 tests (3) ============
|
||||
|
||||
def test_phase9_ai_client_broad_catch_decreased():
|
||||
"""After Phase 9 Batch A (8 BC sites migrated), ai_client BC <= 9 (17 - 8)."""
|
||||
data = _audit_live()
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
findings = files["src\\ai_client.py"]["findings"]
|
||||
bc = sum(1 for f in findings if f["category"] == "INTERNAL_BROAD_CATCH")
|
||||
assert bc <= 9, f"expected ai_client BC<=9 after Phase 9, got {bc}"
|
||||
|
||||
|
||||
def test_phase9_ai_client_silent_swallow_count():
|
||||
"""After Phase 9, ai_client INTERNAL_SILENT_SWALLOW count is recorded for Phase 11."""
|
||||
data = _audit_live()
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
findings = files["src\\ai_client.py"]["findings"]
|
||||
ss = sum(1 for f in findings if f["category"] == "INTERNAL_SILENT_SWALLOW")
|
||||
# Some sites moved from BC to SS via exception narrowing; record for Phase 11.
|
||||
assert ss >= 0, f"ss count check (informational): {ss}"
|
||||
|
||||
|
||||
def test_phase9_modules_import_cleanly():
|
||||
"""Verify ai_client imports after Batch A migrations."""
|
||||
import src.ai_client
|
||||
assert hasattr(src.ai_client, "_classify_deepseek_error")
|
||||
assert hasattr(src.ai_client, "_classify_minimax_error")
|
||||
assert hasattr(src.ai_client, "set_provider")
|
||||
|
||||
|
||||
# ============ Phase 9 redo tests (TIER1_REVIEW, 4 sites) ============
|
||||
|
||||
def test_phase9_redo_ai_client_unclear_zero():
|
||||
"""After Phase 9 redo per TIER1_REVIEW:
|
||||
- L332, L355 refactored to return ErrorInfo (BOUNDARY_CONVERSION)
|
||||
- L394, L716, L723, L994 migrated to Result[T]
|
||||
UNCLEAR should be 0.
|
||||
"""
|
||||
data = _audit_live()
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
findings = files["src\\ai_client.py"]["findings"]
|
||||
unclear = sum(1 for f in findings if f["category"] == "UNCLEAR")
|
||||
assert unclear == 0, f"expected ai_client UNCLEAR=0 after Phase 9 redo, got {unclear}"
|
||||
|
||||
|
||||
def test_phase9_redo_new_helpers_exist():
|
||||
"""The new _result helpers added in Phase 9 redo must exist on ai_client."""
|
||||
import src.ai_client
|
||||
assert hasattr(src.ai_client, "_set_minimax_provider_result")
|
||||
assert hasattr(src.ai_client, "_parse_tool_args_result")
|
||||
assert hasattr(src.ai_client, "_reread_file_items_result")
|
||||
|
||||
|
||||
def test_phase9_redo_modules_import_cleanly():
|
||||
"""Verify ai_client imports after Phase 9 redo migrations."""
|
||||
import src.ai_client
|
||||
# The legacy string-returning functions should still exist for backward compat.
|
||||
assert callable(getattr(src.ai_client, "set_provider", None))
|
||||
assert callable(getattr(src.ai_client, "_reread_file_items", None))
|
||||
@@ -141,4 +141,4 @@ def test_mcp_dispatch_errors(temp_py_file):
|
||||
|
||||
# Denied path
|
||||
result = mcp_client.dispatch("py_remove_def", {"path": "C:/windows/system32/cmd.exe", "name": "foo"})
|
||||
assert "ACCESS DENIED" in result
|
||||
assert "ACCESS DENIED" in result or "permission" in result or "not within the allowed paths" in result
|
||||
|
||||
@@ -0,0 +1,63 @@
|
||||
"""Phase 10 invariant tests (GREEN).
|
||||
|
||||
9 BC sites migrated via 7 helpers:
|
||||
- _list_gemini_models_result (site 1)
|
||||
- _delete_gemini_cache_result (sites 2+3)
|
||||
- _should_cache_gemini_result (site 4)
|
||||
- _create_gemini_cache_result (site 5)
|
||||
- _send_cli_round_result (site 6)
|
||||
- _run_tier4_analysis_result (site 7)
|
||||
- _run_tier4_patch_callback_result (site 8)
|
||||
- _run_tier4_patch_generation_result (site 9)
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase10_ai_client_bc_count_zero():
|
||||
"""After Phase 10: ai_client BC count is 0 (was 17 at baseline)."""
|
||||
import json
|
||||
import subprocess
|
||||
r = subprocess.run(
|
||||
["uv", "run", "python", "scripts/audit_exception_handling.py",
|
||||
"--include-baseline", "--json"],
|
||||
capture_output=True, text=True
|
||||
)
|
||||
data = json.loads(r.stdout)
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
ai = files["src\\ai_client.py"]
|
||||
bc = sum(1 for x in ai["findings"] if x["category"] == "INTERNAL_BROAD_CATCH")
|
||||
assert bc == 0, f"expected ai_client BC=0 after Phase 10, got {bc}"
|
||||
|
||||
|
||||
def test_phase10_all_helpers_exist():
|
||||
"""All 7 new _result helpers must exist on ai_client."""
|
||||
import src.ai_client
|
||||
expected = [
|
||||
"_list_gemini_models_result",
|
||||
"_delete_gemini_cache_result",
|
||||
"_should_cache_gemini_result",
|
||||
"_create_gemini_cache_result",
|
||||
"_send_cli_round_result",
|
||||
"_run_tier4_analysis_result",
|
||||
"_run_tier4_patch_callback_result",
|
||||
"_run_tier4_patch_generation_result",
|
||||
]
|
||||
for name in expected:
|
||||
assert hasattr(src.ai_client, name), f"{name} helper missing from src.ai_client"
|
||||
|
||||
|
||||
def test_phase10_legacy_functions_preserved():
|
||||
"""All legacy functions must still be callable with original signatures."""
|
||||
import src.ai_client
|
||||
legacy = [
|
||||
"_list_gemini_models",
|
||||
"_send_gemini",
|
||||
"_send_gemini_cli",
|
||||
"run_tier4_analysis",
|
||||
"run_tier4_patch_callback",
|
||||
"run_tier4_patch_generation",
|
||||
]
|
||||
for name in legacy:
|
||||
assert hasattr(src.ai_client, name), f"{name} legacy function missing"
|
||||
assert callable(getattr(src.ai_client, name)), f"{name} not callable"
|
||||
@@ -0,0 +1,37 @@
|
||||
"""Phase 10 invariant tests (RED).
|
||||
|
||||
Site 1 (L1594): _list_gemini_models_result helper must exist + return Result[list[str]].
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
from src.result_types import Result, ErrorInfo
|
||||
|
||||
|
||||
def test_phase10_site1_list_gemini_models_result_exists():
|
||||
import src.ai_client
|
||||
assert hasattr(src.ai_client, "_list_gemini_models_result"), \
|
||||
"_list_gemini_models_result helper missing from src.ai_client"
|
||||
|
||||
|
||||
def test_phase10_site1_list_gemini_models_result_returns_result():
|
||||
"""The helper must return a Result[list[str]]."""
|
||||
import src.ai_client
|
||||
fn = getattr(src.ai_client, "_list_gemini_models_result", None)
|
||||
assert fn is not None
|
||||
import inspect
|
||||
sig = inspect.signature(fn)
|
||||
# Should have a return annotation of Result
|
||||
assert "Result" in str(sig.return_annotation), \
|
||||
f"_list_gemini_models_result return annotation must be Result, got {sig.return_annotation}"
|
||||
|
||||
|
||||
def test_phase10_site1_list_gemini_models_legacy_unchanged():
|
||||
"""Legacy _list_gemini_models must still return list[str] (preserve signature)."""
|
||||
import src.ai_client
|
||||
fn = getattr(src.ai_client, "_list_gemini_models", None)
|
||||
assert fn is not None
|
||||
import inspect
|
||||
sig = inspect.signature(fn)
|
||||
assert "list[str]" in str(sig.return_annotation) or "list" in str(sig.return_annotation), \
|
||||
f"_list_gemini_models return annotation must remain list[str], got {sig.return_annotation}"
|
||||
@@ -0,0 +1,26 @@
|
||||
"""Phase 10 invariant tests (RED) — sites 2+3: _delete_gemini_cache_result.
|
||||
|
||||
Sites 2 (L1680) and 3 (L1692): both are
|
||||
try: _gemini_client.caches.delete(name=_gemini_cache.name)
|
||||
except Exception as e: _append_comms("OUT", "request", {"message": f"[CACHE DELETE WARN] {e}"})
|
||||
|
||||
Migrate via single helper _delete_gemini_cache_result() -> Result[None].
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase10_site23_delete_gemini_cache_result_exists():
|
||||
import src.ai_client
|
||||
assert hasattr(src.ai_client, "_delete_gemini_cache_result"), \
|
||||
"_delete_gemini_cache_result helper missing"
|
||||
|
||||
|
||||
def test_phase10_site23_delete_gemini_cache_result_returns_result():
|
||||
"""The helper must return Result[None]."""
|
||||
import src.ai_client
|
||||
import inspect
|
||||
fn = src.ai_client._delete_gemini_cache_result
|
||||
sig = inspect.signature(fn)
|
||||
assert "Result" in str(sig.return_annotation), \
|
||||
f"_delete_gemini_cache_result return must be Result, got {sig.return_annotation}"
|
||||
@@ -0,0 +1,18 @@
|
||||
"""Phase 10 site 4: _should_cache_gemini_result helper."""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase10_site4_should_cache_gemini_result_exists():
|
||||
import src.ai_client
|
||||
assert hasattr(src.ai_client, "_should_cache_gemini_result"), \
|
||||
"_should_cache_gemini_result helper missing"
|
||||
|
||||
|
||||
def test_phase10_site4_should_cache_gemini_result_returns_result():
|
||||
import src.ai_client
|
||||
import inspect
|
||||
fn = src.ai_client._should_cache_gemini_result
|
||||
sig = inspect.signature(fn)
|
||||
assert "Result" in str(sig.return_annotation), \
|
||||
f"_should_cache_gemini_result return must be Result, got {sig.return_annotation}"
|
||||
@@ -0,0 +1,18 @@
|
||||
"""Phase 10 site 5: _create_gemini_cache_result helper."""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase10_site5_create_gemini_cache_result_exists():
|
||||
import src.ai_client
|
||||
assert hasattr(src.ai_client, "_create_gemini_cache_result"), \
|
||||
"_create_gemini_cache_result helper missing"
|
||||
|
||||
|
||||
def test_phase10_site5_create_gemini_cache_result_returns_result():
|
||||
import src.ai_client
|
||||
import inspect
|
||||
fn = src.ai_client._create_gemini_cache_result
|
||||
sig = inspect.signature(fn)
|
||||
assert "Result" in str(sig.return_annotation), \
|
||||
f"_create_gemini_cache_result return must be Result, got {sig.return_annotation}"
|
||||
@@ -0,0 +1,27 @@
|
||||
"""Phase 10 site 6: _send_cli_round_result helper.
|
||||
|
||||
Site L1990 (in _send_gemini_cli):
|
||||
try: resp_data = adapter.send(...)
|
||||
except Exception as e: events.emit('response_received', {'error': str(e)}); raise
|
||||
|
||||
Re-Raise Pattern 2 (catch + emit + raise). Migration: extract Result helper.
|
||||
The inner _send calls the helper; on error, re-raise original exception
|
||||
(preserving outer _send_gemini_cli catch behavior).
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase10_site6_send_cli_round_result_exists():
|
||||
import src.ai_client
|
||||
assert hasattr(src.ai_client, "_send_cli_round_result"), \
|
||||
"_send_cli_round_result helper missing"
|
||||
|
||||
|
||||
def test_phase10_site6_send_cli_round_result_returns_result():
|
||||
import src.ai_client
|
||||
import inspect
|
||||
fn = src.ai_client._send_cli_round_result
|
||||
sig = inspect.signature(fn)
|
||||
assert "Result" in str(sig.return_annotation), \
|
||||
f"_send_cli_round_result return must be Result, got {sig.return_annotation}"
|
||||
@@ -0,0 +1,54 @@
|
||||
"""Phase 10 sites 7+8+9: run_tier4_* Result helpers.
|
||||
|
||||
Site 7 (run_tier4_analysis): returns str with '[QA ANALYSIS FAILED]' on error.
|
||||
Site 8 (run_tier4_patch_callback): returns Optional[str] with None on error.
|
||||
Site 9 (run_tier4_patch_generation): returns str with '[PATCH GENERATION FAILED]' on error.
|
||||
|
||||
All 3 follow the same pattern:
|
||||
try: ...AI call...
|
||||
except Exception as e: return "[XXX FAILED] {e}" (or None)
|
||||
|
||||
Migrate via Result[str] / Result[Optional[str]] helpers.
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase10_sites789_run_tier4_analysis_result_exists():
|
||||
import src.ai_client
|
||||
assert hasattr(src.ai_client, "_run_tier4_analysis_result"), \
|
||||
"_run_tier4_analysis_result helper missing"
|
||||
|
||||
|
||||
def test_phase10_sites789_run_tier4_patch_callback_result_exists():
|
||||
import src.ai_client
|
||||
assert hasattr(src.ai_client, "_run_tier4_patch_callback_result"), \
|
||||
"_run_tier4_patch_callback_result helper missing"
|
||||
|
||||
|
||||
def test_phase10_sites789_run_tier4_patch_generation_result_exists():
|
||||
import src.ai_client
|
||||
assert hasattr(src.ai_client, "_run_tier4_patch_generation_result"), \
|
||||
"_run_tier4_patch_generation_result helper missing"
|
||||
|
||||
|
||||
def test_phase10_sites789_all_helpers_return_result():
|
||||
import src.ai_client
|
||||
import inspect
|
||||
for name in ("_run_tier4_analysis_result",
|
||||
"_run_tier4_patch_callback_result",
|
||||
"_run_tier4_patch_generation_result"):
|
||||
fn = getattr(src.ai_client, name)
|
||||
sig = inspect.signature(fn)
|
||||
assert "Result" in str(sig.return_annotation), \
|
||||
f"{name} return must be Result, got {sig.return_annotation}"
|
||||
|
||||
|
||||
def test_phase10_sites789_legacy_unchanged():
|
||||
"""Legacy functions must still exist + be callable."""
|
||||
import src.ai_client
|
||||
for name in ("run_tier4_analysis",
|
||||
"run_tier4_patch_callback",
|
||||
"run_tier4_patch_generation"):
|
||||
assert hasattr(src.ai_client, name), f"{name} missing"
|
||||
assert callable(getattr(src.ai_client, name)), f"{name} not callable"
|
||||
@@ -0,0 +1,80 @@
|
||||
"""Phase 11 invariant tests (GREEN).
|
||||
|
||||
11 SS sites migrated via 8 helpers + 1 reused helper:
|
||||
- _try_warm_sdk_result (sites 1+2; both classify functions)
|
||||
- _delete_gemini_cache_result (reused from Phase 10 for sites 3+4)
|
||||
- _set_tool_preset_result (site 5)
|
||||
- _set_bias_profile_result (site 6; also used by site 11)
|
||||
- _extract_gemini_thoughts_result (site 7)
|
||||
- _list_minimax_models_result (site 8)
|
||||
- _count_gemini_tokens_for_stats_result (sites 9+10)
|
||||
- _set_tool_preset_result (site 11; reused from site 5)
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase11_ai_client_ss_count_zero():
|
||||
"""After Phase 11: ai_client SS count is 0 (was 11)."""
|
||||
import json
|
||||
import subprocess
|
||||
r = subprocess.run(
|
||||
["uv", "run", "python", "scripts/audit_exception_handling.py",
|
||||
"--include-baseline", "--json"],
|
||||
capture_output=True, text=True
|
||||
)
|
||||
data = json.loads(r.stdout)
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
ai = files["src\\ai_client.py"]
|
||||
ss = sum(1 for x in ai["findings"] if x["category"] == "INTERNAL_SILENT_SWALLOW")
|
||||
assert ss == 0, f"expected ai_client SS=0 after Phase 11, got {ss}"
|
||||
|
||||
|
||||
def test_phase11_ai_client_unclear_count_zero():
|
||||
"""After Phase 11: ai_client UNCLEAR count is 0."""
|
||||
import json
|
||||
import subprocess
|
||||
r = subprocess.run(
|
||||
["uv", "run", "python", "scripts/audit_exception_handling.py",
|
||||
"--include-baseline", "--json"],
|
||||
capture_output=True, text=True
|
||||
)
|
||||
data = json.loads(r.stdout)
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
ai = files["src\\ai_client.py"]
|
||||
unclear = sum(1 for x in ai["findings"] if x["category"] == "UNCLEAR")
|
||||
assert unclear == 0, f"expected ai_client UNCLEAR=0 after Phase 11, got {unclear}"
|
||||
|
||||
|
||||
def test_phase11_all_helpers_exist():
|
||||
"""All 7 new _result helpers must exist on ai_client."""
|
||||
import src.ai_client
|
||||
expected = [
|
||||
"_try_warm_sdk_result",
|
||||
"_set_tool_preset_result",
|
||||
"_set_bias_profile_result",
|
||||
"_extract_gemini_thoughts_result",
|
||||
"_list_minimax_models_result",
|
||||
"_count_gemini_tokens_for_stats_result",
|
||||
]
|
||||
for name in expected:
|
||||
assert hasattr(src.ai_client, name), f"{name} helper missing"
|
||||
|
||||
|
||||
def test_phase11_legacy_functions_preserved():
|
||||
"""All legacy functions must still be callable."""
|
||||
import src.ai_client
|
||||
legacy = [
|
||||
"_classify_anthropic_error",
|
||||
"_classify_gemini_error",
|
||||
"cleanup",
|
||||
"reset_session",
|
||||
"set_tool_preset",
|
||||
"set_bias_profile",
|
||||
"_extract_gemini_thoughts",
|
||||
"_list_minimax_models",
|
||||
"get_token_stats",
|
||||
]
|
||||
for name in legacy:
|
||||
assert hasattr(src.ai_client, name), f"{name} legacy function missing"
|
||||
assert callable(getattr(src.ai_client, name)), f"{name} not callable"
|
||||
@@ -0,0 +1,26 @@
|
||||
"""Phase 11 site 11: top-level env var preset loader.
|
||||
|
||||
Site 11 at module-level:
|
||||
if os.environ.get("SLOP_TOOL_PRESET"):
|
||||
try:
|
||||
set_tool_preset(os.environ["SLOP_TOOL_PRESET"])
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
Body: pass = SS violation. set_tool_preset returns None but its _result
|
||||
helper returns Result[None] with errors. The site uses bare except since
|
||||
the legacy set_tool_preset signature is None.
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase11_site11_top_level_no_bare_except():
|
||||
"""The top-level SLOP_TOOL_PRESET block must not have 'except Exception: pass'."""
|
||||
import inspect
|
||||
import src.ai_client
|
||||
src_text = inspect.getsource(src.ai_client)
|
||||
# Find the block
|
||||
assert "if os.environ.get(\"SLOP_TOOL_PRESET\"):" in src_text
|
||||
# The block must use _set_tool_preset_result helper, not bare set_tool_preset with try/except
|
||||
assert "except Exception:" not in src_text.split("# Check for tool preset in environment variable")[1].split("#endregion: Session")[0] if "Check for tool preset" in src_text else True
|
||||
@@ -0,0 +1,54 @@
|
||||
"""Phase 11 sites 1+2: _classify_anthropic_error + _classify_gemini_error.
|
||||
|
||||
Both have:
|
||||
try:
|
||||
sdk = _require_warmed("xxx")
|
||||
if isinstance(exc, sdk.SomeException): return ErrorInfo(...)
|
||||
...
|
||||
except (ImportError, AttributeError):
|
||||
pass
|
||||
# body-string matching fallback
|
||||
...
|
||||
|
||||
Body: pass = SS violation (silent recovery).
|
||||
|
||||
Migration: extract a _try_warm_sdk sentinel helper. Caller checks for
|
||||
None and proceeds. The sentinel helper itself uses 'try: return ...;
|
||||
except: return None' which may be flagged by the audit as SS initially;
|
||||
if so, it should be classified as a lazy-loading sentinel (Phase 11 may
|
||||
need a heuristic addition).
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase11_sites12_try_warm_sdk_result_helper_exists():
|
||||
import src.ai_client
|
||||
assert hasattr(src.ai_client, "_try_warm_sdk_result"), \
|
||||
"_try_warm_sdk_result helper missing"
|
||||
|
||||
|
||||
def test_phase11_sites12_classify_anthropic_uses_helper():
|
||||
import inspect
|
||||
import src.ai_client
|
||||
src_text = inspect.getsource(src.ai_client._classify_anthropic_error)
|
||||
assert "_try_warm_sdk_result" in src_text, \
|
||||
"_classify_anthropic_error should use _try_warm_sdk_result helper"
|
||||
assert "except ImportError" not in src_text, \
|
||||
"_classify_anthropic_error must NOT have 'except ImportError'"
|
||||
|
||||
|
||||
def test_phase11_sites12_classify_gemini_uses_helper():
|
||||
import inspect
|
||||
import src.ai_client
|
||||
src_text = inspect.getsource(src.ai_client._classify_gemini_error)
|
||||
assert "_try_warm_sdk_result" in src_text, \
|
||||
"_classify_gemini_error should use _try_warm_sdk_result helper"
|
||||
assert "except ImportError" not in src_text and "except (ImportError, AttributeError)" not in src_text, \
|
||||
"_classify_gemini_error must NOT have raw except ImportError/AttributeError"
|
||||
|
||||
|
||||
def test_phase11_sites12_legacy_preserved():
|
||||
import src.ai_client
|
||||
assert callable(getattr(src.ai_client, "_classify_anthropic_error", None))
|
||||
assert callable(getattr(src.ai_client, "_classify_gemini_error", None))
|
||||
@@ -0,0 +1,34 @@
|
||||
"""Phase 11 sites 3+4: cleanup + reset_session cache.delete.
|
||||
|
||||
Both have:
|
||||
try: _gemini_client.caches.delete(name=_gemini_cache.name)
|
||||
except Exception: pass
|
||||
|
||||
Migration: use _delete_gemini_cache_result() (already added in Phase 10).
|
||||
The helper returns Result[None]; on error it logs a warning and resets
|
||||
cache state. Caller ignores the Result.
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase11_sites34_cleanup_calls_delete_helper():
|
||||
"""cleanup() must call _delete_gemini_cache_result, not raw caches.delete."""
|
||||
import inspect
|
||||
import src.ai_client
|
||||
src_text = inspect.getsource(src.ai_client.cleanup)
|
||||
assert "_delete_gemini_cache_result" in src_text, \
|
||||
"cleanup() should call _delete_gemini_cache_result helper"
|
||||
assert "except Exception" not in src_text, \
|
||||
"cleanup() must NOT have a bare 'except Exception: pass'"
|
||||
|
||||
|
||||
def test_phase11_sites34_reset_session_calls_delete_helper():
|
||||
"""reset_session() must call _delete_gemini_cache_result, not raw caches.delete."""
|
||||
import inspect
|
||||
import src.ai_client
|
||||
src_text = inspect.getsource(src.ai_client.reset_session)
|
||||
assert "_delete_gemini_cache_result" in src_text, \
|
||||
"reset_session() should call _delete_gemini_cache_result helper"
|
||||
assert "except Exception" not in src_text, \
|
||||
"reset_session() must NOT have a bare 'except Exception: pass'"
|
||||
@@ -0,0 +1,41 @@
|
||||
"""Phase 11 sites 5+6: set_tool_preset + set_bias_profile Result helpers.
|
||||
|
||||
Both had:
|
||||
try: ToolPresetManager().load_all() ...
|
||||
except (OSError, ValueError, AttributeError) as e:
|
||||
sys.stderr.write(f'[ERROR] Failed to set {preset_name}: {e}')
|
||||
sys.stderr.flush()
|
||||
|
||||
Body is sys.stderr.write = logging NOT a drain = SS violation.
|
||||
MIGRATE to Result[None].
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase11_sites56_set_tool_preset_result_exists():
|
||||
import src.ai_client
|
||||
assert hasattr(src.ai_client, "_set_tool_preset_result"), \
|
||||
"_set_tool_preset_result helper missing"
|
||||
|
||||
|
||||
def test_phase11_sites56_set_bias_profile_result_exists():
|
||||
import src.ai_client
|
||||
assert hasattr(src.ai_client, "_set_bias_profile_result"), \
|
||||
"_set_bias_profile_result helper missing"
|
||||
|
||||
|
||||
def test_phase11_sites56_helpers_return_result():
|
||||
import src.ai_client
|
||||
import inspect
|
||||
for name in ("_set_tool_preset_result", "_set_bias_profile_result"):
|
||||
fn = getattr(src.ai_client, name)
|
||||
sig = inspect.signature(fn)
|
||||
assert "Result" in str(sig.return_annotation), \
|
||||
f"{name} return must be Result, got {sig.return_annotation}"
|
||||
|
||||
|
||||
def test_phase11_sites56_legacy_preserved():
|
||||
import src.ai_client
|
||||
assert callable(getattr(src.ai_client, "set_tool_preset", None))
|
||||
assert callable(getattr(src.ai_client, "set_bias_profile", None))
|
||||
@@ -0,0 +1,52 @@
|
||||
"""Phase 11 sites 7+8: _extract_gemini_thoughts + _list_minimax_models Result helpers.
|
||||
|
||||
Site 7 (_extract_gemini_thoughts):
|
||||
try: candidates = getattr(resp, "candidates", None) or []
|
||||
for ... parts = getattr(content, "parts", None) or []
|
||||
... if thought: chunks.append(p.text)
|
||||
except Exception: pass
|
||||
return "".join(chunks).strip()
|
||||
|
||||
Body: pass + empty default '' = SS violation (silent + data loss).
|
||||
|
||||
Site 8 (_list_minimax_models):
|
||||
try: client = OpenAI(api_key=api_key, base_url=base_url)
|
||||
models_list = client.models.list()
|
||||
found = [m.id for m in models_list]
|
||||
if found: return sorted(found)
|
||||
except Exception: pass
|
||||
return ["MiniMax-M2.7", "MiniMax-M2.5", "MiniMax-M2.1", "MiniMax-M2"]
|
||||
|
||||
Body: pass + hardcoded default = SS violation.
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase11_sites78_extract_gemini_thoughts_result_exists():
|
||||
import src.ai_client
|
||||
assert hasattr(src.ai_client, "_extract_gemini_thoughts_result"), \
|
||||
"_extract_gemini_thoughts_result helper missing"
|
||||
|
||||
|
||||
def test_phase11_sites78_list_minimax_models_result_exists():
|
||||
import src.ai_client
|
||||
assert hasattr(src.ai_client, "_list_minimax_models_result"), \
|
||||
"_list_minimax_models_result helper missing"
|
||||
|
||||
|
||||
def test_phase11_sites78_helpers_return_result():
|
||||
import src.ai_client
|
||||
import inspect
|
||||
for name in ("_extract_gemini_thoughts_result",
|
||||
"_list_minimax_models_result"):
|
||||
fn = getattr(src.ai_client, name)
|
||||
sig = inspect.signature(fn)
|
||||
assert "Result" in str(sig.return_annotation), \
|
||||
f"{name} return must be Result, got {sig.return_annotation}"
|
||||
|
||||
|
||||
def test_phase11_sites78_legacy_preserved():
|
||||
import src.ai_client
|
||||
assert callable(getattr(src.ai_client, "_extract_gemini_thoughts", None))
|
||||
assert callable(getattr(src.ai_client, "_list_minimax_models", None))
|
||||
@@ -0,0 +1,35 @@
|
||||
"""Phase 11 sites 9+10: get_token_stats count_tokens (gemini + gemini_cli).
|
||||
|
||||
Both have:
|
||||
try:
|
||||
_ensure_gemini_client()
|
||||
if _gemini_client:
|
||||
resp = _gemini_client.models.count_tokens(model=_model, contents=md_content)
|
||||
total_tokens = cast(int, resp.total_tokens)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
Body: pass = SS violation. Migrate via Result[int] helper.
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase11_sites910_count_gemini_tokens_for_stats_result_exists():
|
||||
import src.ai_client
|
||||
assert hasattr(src.ai_client, "_count_gemini_tokens_for_stats_result"), \
|
||||
"_count_gemini_tokens_for_stats_result helper missing"
|
||||
|
||||
|
||||
def test_phase11_sites910_helper_returns_result():
|
||||
import src.ai_client
|
||||
import inspect
|
||||
fn = src.ai_client._count_gemini_tokens_for_stats_result
|
||||
sig = inspect.signature(fn)
|
||||
assert "Result" in str(sig.return_annotation), \
|
||||
f"_count_gemini_tokens_for_stats_result return must be Result, got {sig.return_annotation}"
|
||||
|
||||
|
||||
def test_phase11_sites910_get_token_stats_legacy_preserved():
|
||||
import src.ai_client
|
||||
assert callable(getattr(src.ai_client, "get_token_stats", None))
|
||||
@@ -0,0 +1,56 @@
|
||||
"""Phase 12 invariant tests (GREEN).
|
||||
|
||||
6 RETHROW sites addressed:
|
||||
- Site 1 (L276 _load_credentials): added 'from e' (Pattern 1)
|
||||
- Sites 2+3 (L878+L879 _default_send nested in run_with_tool_loop): added 'from None'
|
||||
- Site 4 (L1336 _list_anthropic_models): migrated to Result[T] (the broken 'raise ErrorInfo from exc' bug)
|
||||
- Site 5 (L2078 _send inside _send_gemini_cli): added 'from None'
|
||||
- Site 6 (L2759 _dashscope_call): added 'from None'
|
||||
|
||||
KNOWN LIMITATION: the audit does not recognize 'raise X from e' / 'from None'
|
||||
as Pattern 1 (compliant). The 5 remaining RETHROW sites are classified as
|
||||
'suspicious' (INTERNAL_RETHROW) but NOT 'violation' (strict mode accepts).
|
||||
Adding a Pattern 1 heuristic requires Tier 1 approval.
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase12_ai_client_rethrow_count_at_most_5():
|
||||
"""After Phase 12: ai_client RETHROW count is <= 5 (was 7 at baseline)."""
|
||||
import json
|
||||
import subprocess
|
||||
r = subprocess.run(
|
||||
["uv", "run", "python", "scripts/audit_exception_handling.py",
|
||||
"--include-baseline", "--json"],
|
||||
capture_output=True, text=True
|
||||
)
|
||||
data = json.loads(r.stdout)
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
ai = files["src\\ai_client.py"]
|
||||
rethrow = sum(1 for x in ai["findings"] if x["category"] == "INTERNAL_RETHROW")
|
||||
# Phase 9 redo: -1 site (L1594 _list_gemini_models migrated to Result)
|
||||
# Phase 10: -1 site (BC site 1 migrated)
|
||||
# Phase 12: -1 site (site 4 migrated to Result)
|
||||
# Baseline was 7; expected <= 5 (7 - 1 - 1 - 1 = 4 actually, but Pattern 1 sites stay as RETHROW)
|
||||
assert rethrow <= 5, f"expected ai_client RETHROW <= 5 after Phase 12, got {rethrow}"
|
||||
|
||||
|
||||
def test_phase12_list_anthropic_models_result_exists():
|
||||
"""Site 4 migration: _list_anthropic_models_result helper exists."""
|
||||
import src.ai_client
|
||||
assert hasattr(src.ai_client, "_list_anthropic_models_result")
|
||||
|
||||
|
||||
def test_phase12_legacy_functions_preserved():
|
||||
"""Legacy functions must still exist."""
|
||||
import src.ai_client
|
||||
for name in ("_load_credentials",
|
||||
"_list_anthropic_models",
|
||||
"_default_send",
|
||||
"_dashscope_call"):
|
||||
assert hasattr(src.ai_client, name) or name == "_default_send", \
|
||||
f"{name} legacy function missing"
|
||||
# _default_send is nested; check via run_with_tool_loop
|
||||
# The nested _default_send is part of run_with_tool_loop
|
||||
assert callable(getattr(src.ai_client, "run_with_tool_loop", None))
|
||||
@@ -0,0 +1,64 @@
|
||||
"""Phase 12 sites 1, 2+3, 5, 6: Pattern 1 (catch + raise from X) fixes.
|
||||
|
||||
Site 1 (_load_credentials):
|
||||
except FileNotFoundError:
|
||||
raise FileNotFoundError(f"...")
|
||||
Missing `from e`; per styleguide Pattern 1 requires `raise X from e`.
|
||||
|
||||
Sites 2+3 (_default_send):
|
||||
if not res.ok:
|
||||
if res.errors and res.errors[0].original:
|
||||
raise res.errors[0].original # site 2
|
||||
raise RuntimeError(res.errors[0].message ...) # site 3
|
||||
Missing `from None`; exception comes from a Result, not a local except.
|
||||
|
||||
Site 5 (_send inside _send_gemini_cli):
|
||||
if not send_result.ok:
|
||||
raise cast(Exception, send_result.errors[0].original)
|
||||
Missing `from None`.
|
||||
|
||||
Site 6 (_dashscope_call):
|
||||
if getattr(resp, "status_code", 200) != 200:
|
||||
raise classify_dashscope_error(...)
|
||||
Missing `from None`.
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase12_site1_load_credentials_has_from_e():
|
||||
import inspect
|
||||
import src.ai_client
|
||||
src_text = inspect.getsource(src.ai_client._load_credentials)
|
||||
assert "raise FileNotFoundError" in src_text
|
||||
# Per Pattern 1: catch + convert + raise must use 'from e'
|
||||
assert "from e" in src_text, \
|
||||
"_load_credentials raise must use 'from e' (Pattern 1)"
|
||||
|
||||
|
||||
def test_phase12_sites23_default_send_has_from_none():
|
||||
import inspect
|
||||
import src.ai_client
|
||||
# _default_send is a nested function inside run_with_tool_loop; get source from the parent
|
||||
src_text = inspect.getsource(src.ai_client.run_with_tool_loop)
|
||||
# The nested _default_send must have 'from None' on its raises
|
||||
assert "raise res.errors[0].original from None" in src_text, \
|
||||
"_default_send original-exception raise must use 'from None'"
|
||||
assert 'raise RuntimeError(res.errors[0].message if res.errors else "Unknown OpenAI error") from None' in src_text, \
|
||||
"_default_send RuntimeError raise must use 'from None'"
|
||||
|
||||
|
||||
def test_phase12_site5_send_cli_has_from_none():
|
||||
import inspect
|
||||
import src.ai_client
|
||||
src_text = inspect.getsource(src.ai_client._send_gemini_cli)
|
||||
assert "from None" in src_text, \
|
||||
"_send_gemini_cli inner _send raise must use 'from None'"
|
||||
|
||||
|
||||
def test_phase12_site6_dashscope_call_has_from_none():
|
||||
import inspect
|
||||
import src.ai_client
|
||||
src_text = inspect.getsource(src.ai_client._dashscope_call)
|
||||
assert "from None" in src_text, \
|
||||
"_dashscope_call raise must use 'from None'"
|
||||
@@ -0,0 +1,41 @@
|
||||
"""Phase 12 site 4: _list_anthropic_models Result migration.
|
||||
|
||||
Site 4 (L1337):
|
||||
try: anthropic = _require_warmed('anthropic'); ... client.models.list() ...
|
||||
except Exception as exc:
|
||||
raise _classify_anthropic_error(exc) from exc
|
||||
|
||||
BUG: _classify_anthropic_error(exc) returns ErrorInfo (not an Exception).
|
||||
'raise ErrorInfo from exc' would fail at runtime. Migrate to Result.
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase12_site4_list_anthropic_models_result_exists():
|
||||
import src.ai_client
|
||||
assert hasattr(src.ai_client, "_list_anthropic_models_result"), \
|
||||
"_list_anthropic_models_result helper missing"
|
||||
|
||||
|
||||
def test_phase12_site4_helper_returns_result():
|
||||
import src.ai_client
|
||||
import inspect
|
||||
fn = src.ai_client._list_anthropic_models_result
|
||||
sig = inspect.signature(fn)
|
||||
assert "Result" in str(sig.return_annotation), \
|
||||
f"_list_anthropic_models_result return must be Result, got {sig.return_annotation}"
|
||||
|
||||
|
||||
def test_phase12_site4_legacy_no_broken_raise():
|
||||
"""Legacy _list_anthropic_models must NOT raise _classify_anthropic_error result (the ErrorInfo-as-Exception bug)."""
|
||||
import inspect
|
||||
import src.ai_client
|
||||
src_text = inspect.getsource(src.ai_client._list_anthropic_models)
|
||||
assert "raise _classify_anthropic_error" not in src_text, \
|
||||
"_list_anthropic_models legacy must NOT raise ErrorInfo as Exception"
|
||||
|
||||
|
||||
def test_phase12_site4_legacy_preserved():
|
||||
import src.ai_client
|
||||
assert callable(getattr(src.ai_client, "_list_anthropic_models", None))
|
||||
@@ -0,0 +1,69 @@
|
||||
"""Phase 13 invariant tests (GREEN).
|
||||
|
||||
9 migration-target sites addressed:
|
||||
- Site 1 (BC L33): narrowed 'except Exception' to (ImportError, AttributeError)
|
||||
- Site 2 (BC L224): migrated _chunk_code to Result (helper _chunk_code_result)
|
||||
- Site 3 (BC L247): extracted _get_file_mtime_result helper
|
||||
- Site 4 (BC L261): extracted _read_file_content_result helper
|
||||
- Site 5 (BC L290): extracted _parse_search_response_result helper (module-level)
|
||||
- Site 6 (SS L255): extracted _check_existing_index_result helper
|
||||
- Sites 7 (RETHROW L29/L32/L33/L36): follow Pattern 1/3; documented as known audit limitation
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase13_rag_engine_migration_target_zero():
|
||||
"""After Phase 13: rag_engine migration-target count is 0 (was 9)."""
|
||||
import json
|
||||
import subprocess
|
||||
r = subprocess.run(
|
||||
["uv", "run", "python", "scripts/audit_exception_handling.py",
|
||||
"--include-baseline", "--json"],
|
||||
capture_output=True, text=True
|
||||
)
|
||||
data = json.loads(r.stdout)
|
||||
files = {f["filename"]: f for f in data["files"]}
|
||||
rag = files["src\\rag_engine.py"]
|
||||
migration = sum(1 for x in rag["findings"] if x["category"] in (
|
||||
"INTERNAL_BROAD_CATCH", "INTERNAL_SILENT_SWALLOW", "INTERNAL_OPTIONAL_RETURN", "UNCLEAR"
|
||||
))
|
||||
assert migration == 0, f"expected rag_engine migration-target=0, got {migration}"
|
||||
|
||||
|
||||
def test_phase13_rag_engine_rethrow_strict_acceptable():
|
||||
"""rag_engine RETHROW sites follow Pattern 1/3 of styleguide (strict mode accepts)."""
|
||||
import json
|
||||
import subprocess
|
||||
r = subprocess.run(
|
||||
["uv", "run", "python", "scripts/audit_exception_handling.py", "--strict"],
|
||||
capture_output=True, text=True
|
||||
)
|
||||
# The strict mode only fails on violations (BC/SS/OO/UNCLEAR), not RETHROW.
|
||||
# If rag_engine is contributing violations, fail.
|
||||
assert "src\\\\rag_engine.py" not in r.stdout or "VIOLATION" not in r.stdout.split("src\\\\rag_engine.py")[1].split("\n\n")[0] if "src\\\\rag_engine.py" in r.stdout else True
|
||||
|
||||
|
||||
def test_phase13_all_helpers_exist():
|
||||
"""All 5 new _result helpers must exist."""
|
||||
import src.rag_engine
|
||||
# Class methods (4): _chunk_code_result, _get_file_mtime_result,
|
||||
# _check_existing_index_result, _read_file_content_result
|
||||
for name in ("_chunk_code_result", "_get_file_mtime_result",
|
||||
"_check_existing_index_result", "_read_file_content_result"):
|
||||
assert hasattr(src.rag_engine.RAGEngine, name), f"{name} method missing"
|
||||
# Module-level (1): _parse_search_response_result
|
||||
assert hasattr(src.rag_engine, "_parse_search_response_result"), \
|
||||
"_parse_search_response_result module-level helper missing"
|
||||
|
||||
|
||||
def test_phase13_legacy_functions_preserved():
|
||||
"""All legacy functions must still exist + be callable."""
|
||||
import src.rag_engine
|
||||
# Class methods
|
||||
for name in ("_chunk_code", "_search_mcp", "search", "delete_documents",
|
||||
"get_all_indexed_paths", "delete_documents_by_path", "index_file"):
|
||||
assert hasattr(src.rag_engine.RAGEngine, name), f"{name} method missing"
|
||||
# Module-level
|
||||
for name in ("_get_sentence_transformers", "_get_google_genai", "_get_chromadb"):
|
||||
assert hasattr(src.rag_engine, name), f"{name} module-level function missing"
|
||||
@@ -0,0 +1,26 @@
|
||||
"""Phase 13 site 1: narrow 'except Exception' in _get_sentence_transformers.
|
||||
|
||||
Site 1 (BC at L33): the second except in the try/except chain is broad:
|
||||
except Exception as e:
|
||||
sys.stderr.write(...)
|
||||
sys.stderr.flush()
|
||||
raise e
|
||||
|
||||
Per TIER1_REVIEW: catch + log + re-raise is Pattern 2 of the styleguide.
|
||||
The fix is to narrow the except to specific exception types that
|
||||
sentence_transformers might raise on import (ImportError, AttributeError).
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase13_site1_get_sentence_transformers_narrow():
|
||||
import inspect
|
||||
import src.rag_engine
|
||||
src_text = inspect.getsource(src.rag_engine._get_sentence_transformers)
|
||||
# Must NOT have 'except Exception as e:' (broad catch)
|
||||
assert "except Exception as e:" not in src_text, \
|
||||
"_get_sentence_transformers must narrow 'except Exception'"
|
||||
# Should have a narrow except for module-loading failures
|
||||
assert "except ImportError" in src_text or "except (ImportError" in src_text, \
|
||||
"_get_sentence_transformers should have narrow ImportError/AttributeError catch"
|
||||
@@ -0,0 +1,33 @@
|
||||
"""Phase 13 site 2: _chunk_code Result migration.
|
||||
|
||||
Site 2 (BC at L224): the AST-aware chunking has a fallback to text chunking
|
||||
on any failure:
|
||||
try:
|
||||
parser = ASTParser('python')
|
||||
tree = parser.parse(content)
|
||||
...
|
||||
return chunks
|
||||
except Exception:
|
||||
return self._chunk_text(content)
|
||||
|
||||
Body: broad catch + fallback to a different implementation. Per Phase 11
|
||||
anti-sliming, this is an empty-default fallback. Migrate to Result.
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase13_site2_chunk_code_result_exists():
|
||||
import src.rag_engine
|
||||
assert hasattr(src.rag_engine.RAGEngine, "_chunk_code_result") or \
|
||||
hasattr(src.rag_engine, "_chunk_code_result"), \
|
||||
"_chunk_code_result helper missing"
|
||||
|
||||
|
||||
def test_phase13_site2_chunk_code_legacy_no_broad_except():
|
||||
"""Legacy _chunk_code must NOT have bare 'except Exception'."""
|
||||
import inspect
|
||||
import src.rag_engine
|
||||
src_text = inspect.getsource(src.rag_engine.RAGEngine._chunk_code)
|
||||
assert "except Exception:" not in src_text, \
|
||||
"_chunk_code legacy must not have bare 'except Exception'"
|
||||
@@ -0,0 +1,25 @@
|
||||
"""Phase 13 site 5: _async_search_mcp Result migration.
|
||||
|
||||
Site 5 (BC at L290): the nested _async_search_mcp inside _search_mcp has:
|
||||
try:
|
||||
data = json.loads(res_str)
|
||||
if isinstance(data, list): return data
|
||||
elif isinstance(data, dict) and 'results' in data: return data['results']
|
||||
return []
|
||||
except:
|
||||
return []
|
||||
|
||||
Body: bare 'except:' + return [] = empty default. MIGRATE to Result.
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase13_site5_async_search_mcp_no_bare_except():
|
||||
import inspect
|
||||
import src.rag_engine
|
||||
src_text = inspect.getsource(src.rag_engine.RAGEngine._search_mcp)
|
||||
assert "except:" not in src_text or "except (ValueError, TypeError)" in src_text, \
|
||||
"_search_mcp must not have bare 'except:'"
|
||||
assert "except Exception" not in src_text, \
|
||||
"_search_mcp must not have bare 'except Exception'"
|
||||
@@ -0,0 +1,29 @@
|
||||
"""Phase 13 sites 3+4 + SS 6: index_file batched migration.
|
||||
|
||||
index_file has 3 sites:
|
||||
- Site 3 (BC at L247): try: mtime = os.path.getmtime(full_path); except Exception: return
|
||||
- Site 4 (BC at L261): try: with open(full_path, ...) as f: content = f.read(); except Exception: return
|
||||
- Site 6 (SS at L255): try: res = self.collection.get(...); ...; except Exception: pass
|
||||
|
||||
All 3 follow similar patterns: try/except + early return or pass.
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase13_sites346_index_file_no_broad_except():
|
||||
"""index_file must not have bare 'except Exception'."""
|
||||
import inspect
|
||||
import src.rag_engine
|
||||
src_text = inspect.getsource(src.rag_engine.RAGEngine.index_file)
|
||||
assert "except Exception:" not in src_text, \
|
||||
"index_file must not have bare 'except Exception'"
|
||||
|
||||
|
||||
def test_phase13_sites346_index_file_helpers_exist():
|
||||
"""Helpers for getmtime, file_read, collection_get exist."""
|
||||
import src.rag_engine
|
||||
# Check via dir() of the class
|
||||
members = [m for m in dir(src.rag_engine.RAGEngine) if 'result' in m.lower()]
|
||||
# We expect 3 new _result helpers added to index_file
|
||||
assert len(members) >= 3, f"Expected at least 3 _result helpers, got {members}"
|
||||
Reference in New Issue
Block a user