refactor(commands): use lazy registry proxy to defer src.command_palette import
Phase 5A T5A.1-T5A.4 of startup_speedup_20260606 track. src/commands.py was importing src.command_palette at module load to create the CommandRegistry singleton. The 32 @registry.register decorators on the command functions needed this registry at import time. Approach: lazy registry proxy. The @registry.register decorator now just queues the function in a list; the real CommandRegistry is built on first access to any other registry attribute (.all, .get, etc.). By that time, all 32 decorators have run and the pending list is populated, so the real registration is complete in one pass. src/commands.py changes: - Removed 'from src.command_palette import CommandRegistry' - Added 'from src.module_loader import _require_warmed' - Added _LazyCommandRegistry class (proxy) - Added _get_real_registry() function (initializes on first access) - Replaced 'registry = CommandRegistry()' with 'registry = _LazyCommandRegistry()' - The 32 @registry.register decorators are unchanged (the proxy's register method returns the function unchanged after queueing it) EFFECTIVENESS: - 'import src.commands' no longer triggers src.command_palette (~244ms) - The warmup on AppController's _io_pool pre-loads src.command_palette on a background thread during startup - First access to registry.all() (e.g. from gui_2.py at palette open time) is O(1) - the warmup module is already in sys.modules TESTS: - tests/test_commands_no_top_level_command_palette.py: 4/4 PASS (3 RED, 1 green; now all green) - tests/test_command_palette.py: 13/13 PASS (no breakage) - tests/test_command_palette_sim.py: 7/7 PASS (live_gui tests, the full palette flow works end-to-end with the lazy proxy) ARCHITECTURAL NOTE: The lazy proxy is a minimal-change solution that preserves the public API. The 32 decorated functions don't need any changes; gui_2.py's 'from src.commands import registry' still works unchanged. The deferral is invisible to consumers. NEXT: Phase 5B (NERV theme) and 5C (markdown table) follow the same TDD pattern. 5D is the bulk refactor of src/gui_2.py feature-gated imports via the audit_gui2_imports.py script.
This commit is contained in:
+42
-3
@@ -3,18 +3,57 @@ from __future__ import annotations
|
||||
import webbrowser
|
||||
|
||||
from pathlib import Path
|
||||
from typing import TYPE_CHECKING, Callable
|
||||
from typing import TYPE_CHECKING, Any, Callable
|
||||
|
||||
from src import models
|
||||
from src import theme_2
|
||||
from src.module_loader import _require_warmed
|
||||
|
||||
from src.command_palette import CommandRegistry
|
||||
from src.hot_reloader import HotReloader
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from src.gui_2 import App
|
||||
|
||||
registry = CommandRegistry()
|
||||
# Lazy command registry (startup_speedup_20260606 Phase 5A)
|
||||
# --------------------------------------------------------------------------
|
||||
# The @registry.register decorator runs at module import time, but we want
|
||||
# to defer the actual CommandRegistry creation (and the underlying
|
||||
# src.command_palette import, ~244ms) until the palette is actually used.
|
||||
# The proxy below makes @registry.register a no-op that just queues the
|
||||
# function; the real CommandRegistry is built lazily on first access to
|
||||
# any other registry attribute (.all, .get, etc.) by gui_2.py or tests.
|
||||
# --------------------------------------------------------------------------
|
||||
_PENDING_REGISTRATIONS: list[Callable] = []
|
||||
_real_registry: Any = None
|
||||
|
||||
|
||||
class _LazyCommandRegistry:
|
||||
"""Proxy that defers CommandRegistry instantiation.
|
||||
|
||||
Behaves like a CommandRegistry from the caller's perspective:
|
||||
- @registry.register decorates functions by queuing them
|
||||
- .all, .get, etc. trigger real initialization on first access
|
||||
"""
|
||||
|
||||
def register(self, command_or_callable: Any) -> Any:
|
||||
_PENDING_REGISTRATIONS.append(command_or_callable)
|
||||
return command_or_callable
|
||||
|
||||
def __getattr__(self, name: str) -> Any:
|
||||
return getattr(_get_real_registry(), name)
|
||||
|
||||
|
||||
def _get_real_registry() -> Any:
|
||||
global _real_registry
|
||||
if _real_registry is None:
|
||||
command_palette = _require_warmed("src.command_palette")
|
||||
_real_registry = command_palette.CommandRegistry()
|
||||
for func in _PENDING_REGISTRATIONS:
|
||||
_real_registry.register(func)
|
||||
return _real_registry
|
||||
|
||||
|
||||
registry = _LazyCommandRegistry()
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
@@ -0,0 +1,118 @@
|
||||
"""Tests that src/commands.py has NO top-level src.command_palette import.
|
||||
|
||||
Per spec.md:2.2 Layer 1, the main thread's import chain must not include
|
||||
heavy feature-gated modules. src.command_palette (~244ms) is warmed on
|
||||
AppController's _io_pool and accessed via _require_warmed at use sites.
|
||||
|
||||
src/commands.py is a particularly tricky case: it has 32 `@registry.register`
|
||||
decorators on its command functions. The naive "drop the top-level import"
|
||||
approach would break the decorators (they need a registry at module load time).
|
||||
|
||||
Solution: a lazy registry proxy. The @registry.register decorator becomes a
|
||||
no-op that queues the function; the real CommandRegistry is created on first
|
||||
attribute access to the proxy (e.g. registry.all, registry.get). The 32
|
||||
decorated functions get registered at first use, which is the user's first
|
||||
Ctrl+Shift+P press (or any other access to the palette).
|
||||
|
||||
These tests run in a fresh subprocess to ensure no warmup state leaks
|
||||
from the test runner. We assert:
|
||||
- `src.command_palette` is NOT in `sys.modules` after `import src.commands`
|
||||
- The lazy registry proxy works: `from src.commands import registry` succeeds
|
||||
- Accessing `registry.all()` triggers the real CommandRegistry and
|
||||
returns all 32 registered commands
|
||||
- The static audit script reports NO new violation from src/commands.py
|
||||
"""
|
||||
|
||||
import subprocess
|
||||
import sys
|
||||
import textwrap
|
||||
from pathlib import Path
|
||||
|
||||
ROOT = Path(__file__).resolve().parent.parent
|
||||
|
||||
|
||||
def _run_in_subprocess(snippet: str) -> subprocess.CompletedProcess:
|
||||
script = textwrap.dedent(snippet)
|
||||
return subprocess.run(
|
||||
[sys.executable, "-c", script],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
cwd=str(ROOT),
|
||||
timeout=30,
|
||||
)
|
||||
|
||||
|
||||
def test_commands_does_not_import_command_palette_at_module_level() -> None:
|
||||
res = _run_in_subprocess("""
|
||||
import sys
|
||||
import src.commands
|
||||
print('src.command_palette' in sys.modules)
|
||||
""")
|
||||
assert res.returncode == 0, f"stderr: {res.stderr}"
|
||||
assert res.stdout.strip() == "False", f"src.commands triggered src.command_palette import: {res.stdout}"
|
||||
|
||||
|
||||
def test_commands_lazy_registry_proxies_to_real_registry() -> None:
|
||||
"""Accessing registry.all() should trigger real init and return registered commands."""
|
||||
res = _run_in_subprocess("""
|
||||
from src.commands import registry
|
||||
# Access .all() triggers real CommandRegistry creation
|
||||
all_cmds = registry.all()
|
||||
print(len(list(all_cmds)))
|
||||
# After access, src.command_palette SHOULD be in sys.modules
|
||||
import sys
|
||||
print('src.command_palette' in sys.modules)
|
||||
""")
|
||||
assert res.returncode == 0, f"stderr: {res.stderr}"
|
||||
lines = res.stdout.strip().splitlines()
|
||||
# Should have at least 32 commands registered (matches the 32 @registry.register)
|
||||
assert int(lines[0]) >= 32, f"Expected >=32 commands, got {lines[0]}"
|
||||
assert lines[1] == "True", f"src.command_palette should be loaded after registry access, got {lines[1]}"
|
||||
|
||||
|
||||
def test_commands_register_decorator_is_lazy() -> None:
|
||||
"""The @registry.register decorator should NOT trigger command_palette import at module load."""
|
||||
res = _run_in_subprocess("""
|
||||
# Fresh subprocess, just import commands
|
||||
import sys
|
||||
import src.commands
|
||||
# Verify decorator ran but did not trigger command_palette
|
||||
# (the lazy proxy just queues; real init is deferred)
|
||||
print('src.command_palette' in sys.modules)
|
||||
# Verify the function references still exist
|
||||
from src.commands import toggle_command_palette
|
||||
print(callable(toggle_command_palette))
|
||||
""")
|
||||
assert res.returncode == 0, f"stderr: {res.stderr}"
|
||||
lines = res.stdout.strip().splitlines()
|
||||
assert lines[0] == "False", f"Decorator should not trigger command_palette, got {lines[0]}"
|
||||
assert lines[1] == "True", f"toggle_command_palette should be a callable, got {lines[1]}"
|
||||
|
||||
|
||||
def test_audit_main_thread_imports_sees_no_new_violation_from_commands() -> None:
|
||||
"""Run the static audit and check that src/commands.py contributes no
|
||||
new command_palette violations.
|
||||
"""
|
||||
res = _run_in_subprocess("""
|
||||
import ast
|
||||
from pathlib import Path
|
||||
root = Path('.').resolve()
|
||||
commands_path = root / 'src' / 'commands.py'
|
||||
tree = ast.parse(commands_path.read_text(encoding='utf-8'))
|
||||
heavy = ['src.command_palette', 'command_palette']
|
||||
for node in tree.body:
|
||||
if isinstance(node, ast.Import):
|
||||
for alias in node.names:
|
||||
for h in heavy:
|
||||
if alias.name == h or alias.name.startswith(h + '.'):
|
||||
print('VIOLATION:', alias.name)
|
||||
elif isinstance(node, ast.ImportFrom):
|
||||
if node.module:
|
||||
for h in heavy:
|
||||
if node.module == h or node.module.startswith(h + '.'):
|
||||
print('VIOLATION:', node.module)
|
||||
print('OK')
|
||||
""")
|
||||
assert res.returncode == 0, f"stderr: {res.stderr}"
|
||||
assert "OK" in res.stdout
|
||||
assert "VIOLATION" not in res.stdout
|
||||
Reference in New Issue
Block a user