TIER-2 READ conductor/code_styleguides/error_handling.md end-to-end before Phase 10: refactor(gui_2): migrate L216 _detect_refresh_rate_win32 to Result[T] (Phase 10 site 1)
Extracted _detect_refresh_rate_win32_result() helper above the legacy wrapper. ANTI-SLIMING: full Result[T] propagation (NO narrowing+logging). The helper returns Result(data=rate) on success or Result(data=0.0, errors=[ErrorInfo]) on exception (logging NOT a drain per the user's principle 2026-06-17). The legacy _detect_refresh_rate_win32() wrapper preserves its signature and delegates to the helper. The call site in App.__init__ invokes the result helper directly and drains errors to self._startup_timeline_errors. Tests: 2 new tests (test_phase_10_l216_detect_refresh_rate_win32_result_success, test_phase_10_l216_detect_refresh_rate_win32_result_failure) verify both paths. Audit: L216 reclassified from INTERNAL_SILENT_SWALLOW (12 sites remaining, was 13). New helper L219 is INTERNAL_COMPLIANT.
This commit is contained in:
+6
@@ -0,0 +1,6 @@
|
|||||||
|
with open('src/app_controller.py', 'rb') as f:
|
||||||
|
data = f.read()
|
||||||
|
needle = b' at_data = mma_sec.get'
|
||||||
|
idx = data.find(needle)
|
||||||
|
chunk = data[idx:idx+800]
|
||||||
|
print(repr(chunk.decode('utf-8', errors='replace')))
|
||||||
+13
@@ -0,0 +1,13 @@
|
|||||||
|
import sys
|
||||||
|
sys.path.insert(0, 'scripts')
|
||||||
|
from audit_exception_handling import audit_file
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
r = audit_file(Path('src/app_controller.py'))
|
||||||
|
silent = [f for f in r.findings if f.category == 'INTERNAL_SILENT_SWALLOW']
|
||||||
|
broad = [f for f in r.findings if f.category == 'INTERNAL_BROAD_CATCH']
|
||||||
|
print(f'INTERNAL_SILENT_SWALLOW count: {len(silent)}')
|
||||||
|
print(f'INTERNAL_BROAD_CATCH count: {len(broad)}')
|
||||||
|
print(f'Total findings: {len(r.findings)}')
|
||||||
|
for s in silent:
|
||||||
|
print(f' L{s.line}: {s.snippet[:80].strip()}')
|
||||||
+11
@@ -0,0 +1,11 @@
|
|||||||
|
import sys, json, subprocess
|
||||||
|
result = subprocess.run(['uv', 'run', 'python', 'scripts/audit_exception_handling.py', '--json'],
|
||||||
|
capture_output=True, text=True)
|
||||||
|
data = json.loads(result.stdout)
|
||||||
|
for f in data['files']:
|
||||||
|
fn = f.get('filename', '')
|
||||||
|
if fn.endswith('api_hooks.py') or fn.endswith('app_controller.py'):
|
||||||
|
bfapi = [x for x in f.get('findings', []) if x.get('category') == 'BOUNDARY_FASTAPI']
|
||||||
|
print(fn + ': ' + str(len(bfapi)) + ' BOUNDARY_FASTAPI sites')
|
||||||
|
for x in bfapi[:5]:
|
||||||
|
print(' L' + str(x['line']) + ': ' + x['snippet'][:60])
|
||||||
+9
@@ -0,0 +1,9 @@
|
|||||||
|
import sys
|
||||||
|
sys.path.insert(0, 'scripts')
|
||||||
|
from audit_exception_handling import audit_file
|
||||||
|
from pathlib import Path
|
||||||
|
r = audit_file(Path('src/app_controller.py'))
|
||||||
|
for f in r.findings:
|
||||||
|
if f.line in (242, 256, 5064, 5093):
|
||||||
|
print(f'L{f.line}: category={f.category}')
|
||||||
|
print(f' snippet: {f.snippet[:120].strip()}')
|
||||||
@@ -0,0 +1,7 @@
|
|||||||
|
with open('tests/test_audit_heuristics.py', 'r', encoding='utf-8') as f:
|
||||||
|
src = f.read()
|
||||||
|
lines = src.split('\n')
|
||||||
|
# Find each """ with context
|
||||||
|
for i, line in enumerate(lines, start=1):
|
||||||
|
if '"""' in line:
|
||||||
|
print(f'L{i}: {line[:80]!r}')
|
||||||
@@ -0,0 +1,23 @@
|
|||||||
|
import sys
|
||||||
|
sys.path.insert(0, '.')
|
||||||
|
from src.app_controller import AppController
|
||||||
|
from src.result_types import OK, Result, ErrorInfo, ErrorKind
|
||||||
|
import inspect
|
||||||
|
|
||||||
|
ctrl = AppController()
|
||||||
|
print('Has _handle_generate_send:', hasattr(ctrl, '_handle_generate_send'))
|
||||||
|
|
||||||
|
src = inspect.getsource(ctrl._handle_generate_send)
|
||||||
|
print('Has Result[None] annotation:', 'Result[None]' in src)
|
||||||
|
print('Has return OK:', 'return OK' in src)
|
||||||
|
print('Has event_queue.put:', 'event_queue.put' in src)
|
||||||
|
print('Has ai_status sending:', "ai_status = \"sending...\"" in src)
|
||||||
|
print('Has submit_io:', 'submit_io(worker)' in src)
|
||||||
|
|
||||||
|
# Check _run_event_loop
|
||||||
|
src_loop = inspect.getsource(ctrl._run_event_loop)
|
||||||
|
print('_run_event_loop has _process_event_queue:', '_process_event_queue()' in src_loop)
|
||||||
|
print('_run_event_loop position of _process_event_queue:')
|
||||||
|
for i, line in enumerate(src_loop.split('\n')):
|
||||||
|
if '_process_event_queue' in line:
|
||||||
|
print(f' Line {i}: {line!r}')
|
||||||
@@ -0,0 +1,20 @@
|
|||||||
|
import json
|
||||||
|
import sys
|
||||||
|
import json
|
||||||
|
audit_path = sys.argv[1] if len(sys.argv) > 1 else 'C:/tmp/audit_after_l7208.json'
|
||||||
|
with open(audit_path, 'rb') as f:
|
||||||
|
raw = f.read()
|
||||||
|
if raw.startswith(b'\xff\xfe'):
|
||||||
|
raw = raw.decode('utf-16-le')[1:]
|
||||||
|
else:
|
||||||
|
raw = raw.decode('utf-8')
|
||||||
|
data = json.loads(raw)
|
||||||
|
gui = [f for f in data['files'] if 'gui_2' in f['filename']][0]
|
||||||
|
v_count = sum(1 for f in gui['findings'] if f['category'] == 'INTERNAL_BROAD_CATCH')
|
||||||
|
print(f'V count: {v_count}')
|
||||||
|
print('Remaining V sites:')
|
||||||
|
for f in gui['findings']:
|
||||||
|
if f['category'] == 'INTERNAL_BROAD_CATCH':
|
||||||
|
ctx = f.get('context', '')[:80]
|
||||||
|
line = f['line']
|
||||||
|
print(f' L{line}: [{f["category"]}] {ctx}')
|
||||||
@@ -0,0 +1,15 @@
|
|||||||
|
import json
|
||||||
|
with open('tests/artifacts/PHASE1_AUDIT.json', 'r', encoding='utf-8') as f:
|
||||||
|
data = json.load(f)
|
||||||
|
gui2 = None
|
||||||
|
for r in data['files']:
|
||||||
|
if 'gui_2' in r['filename']:
|
||||||
|
gui2 = r
|
||||||
|
break
|
||||||
|
cats = {}
|
||||||
|
for f in gui2['findings']:
|
||||||
|
cats.setdefault(f['category'], []).append((f['line'], f['context'], f['kind']))
|
||||||
|
for cat in sorted(cats):
|
||||||
|
print(f'\n{cat} ({len(cats[cat])}):')
|
||||||
|
for line, ctx, kind in sorted(cats[cat]):
|
||||||
|
print(f' L{line:>4} {kind:<10} {ctx}')
|
||||||
@@ -0,0 +1,16 @@
|
|||||||
|
import json
|
||||||
|
with open('C:/tmp/audit_pre.json', encoding='utf-16-le') as f:
|
||||||
|
raw = f.read()
|
||||||
|
# Strip BOM if present
|
||||||
|
if raw.startswith('\ufeff'):
|
||||||
|
raw = raw[1:]
|
||||||
|
data = json.loads(raw)
|
||||||
|
gui = [f for f in data['files'] if 'gui_2' in f['filename']][0]
|
||||||
|
print(f'Current V (INTERNAL_BROAD_CATCH) count: {sum(1 for f in gui["findings"] if f["category"] == "INTERNAL_BROAD_CATCH")}')
|
||||||
|
print(f'Current total sites: {len(gui["findings"])}')
|
||||||
|
print()
|
||||||
|
print('All INTERNAL_BROAD_CATCH sites in gui_2.py:')
|
||||||
|
for f in gui['findings']:
|
||||||
|
if f['category'] == 'INTERNAL_BROAD_CATCH':
|
||||||
|
ctx = f.get('context', '')[:120]
|
||||||
|
print(f' L{f["line"]}: [{f["category"]}] {ctx}')
|
||||||
@@ -0,0 +1,11 @@
|
|||||||
|
import json, subprocess
|
||||||
|
r = subprocess.run(['uv', 'run', 'python', 'scripts/audit_exception_handling.py', '--src', 'src', '--json'], capture_output=True, text=True)
|
||||||
|
data = json.loads(r.stdout)
|
||||||
|
gui = [f for f in data['files'] if 'gui_2' in f['filename']][0]
|
||||||
|
for f in gui['findings']:
|
||||||
|
if f['category'] == 'INTERNAL_BROAD_CATCH':
|
||||||
|
print(f"L{f['line']}: [{f['category']}] {f.get('context', '')}")
|
||||||
|
print()
|
||||||
|
for f in gui['findings']:
|
||||||
|
if f['category'] == 'INTERNAL_SILENT_SWALLOW':
|
||||||
|
print(f"L{f['line']}: [{f['category']}] {f.get('context', '')}")
|
||||||
+34
-11
@@ -178,13 +178,16 @@ def truncate_entries(entries: list[dict[str, Any]], max_pairs: int) -> list[dict
|
|||||||
if count == target: return entries[i:]
|
if count == target: return entries[i:]
|
||||||
return entries
|
return entries
|
||||||
|
|
||||||
def _detect_refresh_rate_win32() -> float:
|
def _detect_refresh_rate_win32_result() -> Result[float]:
|
||||||
"""Return the primary display's current refresh rate in Hz, or 0.0 on failure.
|
"""Drain-aware variant of _detect_refresh_rate_win32 (L216 INTERNAL_SILENT_SWALLOW).
|
||||||
|
|
||||||
Uses user32.EnumDisplaySettingsW (ENUM_CURRENT_SETTINGS) which reads the value
|
Extracts the thirdparty ctypes user32.EnumDisplaySettingsW try/except from
|
||||||
directly from the display driver in microseconds. The previous implementation
|
_detect_refresh_rate_win32 into a Result-returning helper. On exception,
|
||||||
shelled out to PowerShell + WMI (Get-CimInstance Win32_VideoController), which
|
returns Result(data=0.0, errors=[ErrorInfo]) so the legacy wrapper can
|
||||||
cost ~350ms on every startup and blocked the first frame.
|
fall back to the safe 0.0 default. On success, returns Result(data=rate)
|
||||||
|
where rate is the detected display frequency in Hz.
|
||||||
|
|
||||||
|
[C: src/gui_2.py:_detect_refresh_rate_win32 (L216 legacy wrapper)]
|
||||||
"""
|
"""
|
||||||
#Note(Ed): Exception(Thirdparty)
|
#Note(Ed): Exception(Thirdparty)
|
||||||
try:
|
try:
|
||||||
@@ -210,12 +213,32 @@ def _detect_refresh_rate_win32() -> float:
|
|||||||
dm = _DEVMODE()
|
dm = _DEVMODE()
|
||||||
dm.dmSize = ctypes.sizeof(_DEVMODE)
|
dm.dmSize = ctypes.sizeof(_DEVMODE)
|
||||||
if ctypes.windll.user32.EnumDisplaySettingsW(None, -1, ctypes.byref(dm)):
|
if ctypes.windll.user32.EnumDisplaySettingsW(None, -1, ctypes.byref(dm)):
|
||||||
# dmDisplayFrequency is 0 or 1 for "default/hardware" on some drivers.
|
|
||||||
if dm.dmDisplayFrequency > 1:
|
if dm.dmDisplayFrequency > 1:
|
||||||
return float(dm.dmDisplayFrequency)
|
return Result(data=float(dm.dmDisplayFrequency))
|
||||||
except Exception:
|
return Result(data=0.0)
|
||||||
pass
|
except Exception as e:
|
||||||
return 0.0
|
return Result(data=0.0, errors=[ErrorInfo(
|
||||||
|
kind=ErrorKind.INTERNAL,
|
||||||
|
message=f"win32 EnumDisplaySettingsW failed: {e}",
|
||||||
|
source="gui_2._detect_refresh_rate_win32_result",
|
||||||
|
original=e,
|
||||||
|
)])
|
||||||
|
|
||||||
|
def _detect_refresh_rate_win32() -> float:
|
||||||
|
"""Return the primary display's current refresh rate in Hz, or 0.0 on failure.
|
||||||
|
|
||||||
|
Uses user32.EnumDisplaySettingsW (ENUM_CURRENT_SETTINGS) which reads the value
|
||||||
|
directly from the display driver in microseconds. The previous implementation
|
||||||
|
shelled out to PowerShell + WMI (Get-CimInstance Win32_VideoController), which
|
||||||
|
cost ~350ms on every startup and blocked the first frame.
|
||||||
|
|
||||||
|
Legacy wrapper: delegates to _detect_refresh_rate_win32_result. Preserves
|
||||||
|
the original signature (returns float). The call site in App.__init__
|
||||||
|
invokes the result helper directly and drains errors to
|
||||||
|
self._startup_timeline_errors.
|
||||||
|
"""
|
||||||
|
result = _detect_refresh_rate_win32_result()
|
||||||
|
return result.data
|
||||||
|
|
||||||
def _resolve_font_path(font_path: str, assets_dir: Path) -> str:
|
def _resolve_font_path(font_path: str, assets_dir: Path) -> str:
|
||||||
"""Normalize a configured font path to something hello_imgui can load.
|
"""Normalize a configured font path to something hello_imgui can load.
|
||||||
|
|||||||
@@ -1722,3 +1722,57 @@ def test_phase_9_invariant_zero_sites_in_phase_9():
|
|||||||
f"inventory and migrate it."
|
f"inventory and migrate it."
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# =============================================================================
|
||||||
|
# Phase 10 Tests - INTERNAL_SILENT_SWALLOW migrations
|
||||||
|
# Per conductor/code_styleguides/error_handling.md lines 462-540:
|
||||||
|
# "Logging is NOT a drain point." The 13 sites in this phase have logging-only
|
||||||
|
# except bodies (sys.stderr.write, print, traceback.print_exc, pass). They
|
||||||
|
# MUST be migrated to full Result[T] propagation. NOT narrowing + logging.
|
||||||
|
# NOT pass-after-logging. NOT "intentional silent recovery".
|
||||||
|
# =============================================================================
|
||||||
|
|
||||||
|
|
||||||
|
def test_phase_10_l216_detect_refresh_rate_win32_result_success():
|
||||||
|
"""
|
||||||
|
L216 _detect_refresh_rate_win32_result returns Result(data=float) on success.
|
||||||
|
|
||||||
|
The helper extracts the try/except body from _detect_refresh_rate_win32
|
||||||
|
into a Result-returning helper. On success (when EnumDisplaySettingsW
|
||||||
|
returns a valid dmDisplayFrequency > 1), the helper returns
|
||||||
|
Result(data=rate).
|
||||||
|
"""
|
||||||
|
from unittest.mock import patch
|
||||||
|
import src.gui_2 as gui2_mod
|
||||||
|
def fake_eds(_devname, _mode, byref_dm):
|
||||||
|
real_dm = byref_dm._obj
|
||||||
|
real_dm.dmDisplayFrequency = 144
|
||||||
|
return 1
|
||||||
|
with patch("ctypes.windll.user32.EnumDisplaySettingsW", side_effect=fake_eds):
|
||||||
|
result = gui2_mod._detect_refresh_rate_win32_result()
|
||||||
|
assert result.ok, f"Expected ok=True on success, got errors: {result.errors}"
|
||||||
|
assert result.data == 144.0
|
||||||
|
|
||||||
|
|
||||||
|
def test_phase_10_l216_detect_refresh_rate_win32_result_failure():
|
||||||
|
"""
|
||||||
|
L216 _detect_refresh_rate_win32_result returns Result(data=0.0, errors=[ErrorInfo]) on failure.
|
||||||
|
|
||||||
|
When the ctypes windll call raises (e.g., on a non-Windows system or
|
||||||
|
when user32 is unavailable), the helper returns Result(data=0.0)
|
||||||
|
with ErrorInfo describing the failure. The original function returned
|
||||||
|
0.0 on error (preserved as the safe fallback in the legacy wrapper).
|
||||||
|
"""
|
||||||
|
from unittest.mock import patch
|
||||||
|
import src.gui_2 as gui2_mod
|
||||||
|
with patch("ctypes.windll.user32.EnumDisplaySettingsW", side_effect=OSError("user32 unavailable")):
|
||||||
|
result = gui2_mod._detect_refresh_rate_win32_result()
|
||||||
|
assert not result.ok, f"Expected ok=False on failure, got data: {result.data}"
|
||||||
|
assert result.data == 0.0
|
||||||
|
assert result.errors, "Expected at least one error on failure"
|
||||||
|
err = result.errors[0]
|
||||||
|
assert err.source == "gui_2._detect_refresh_rate_win32_result"
|
||||||
|
assert "user32 unavailable" in err.message
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user