Private
Public Access
refactor(gui_2): merge command_palette; split registry->commands + render->gui_2; git rm src/command_palette.py
Per spec FR1 + Phase 1.3 + architecture feedback: src/command_palette.py
split by responsibility:
- Command/ScoredCommand/CommandRegistry/fuzzy_match/_close_palette/_execute (data/ops)
-> src/commands.py (which already owns _LazyCommandRegistry pattern)
- render_palette_modal (view/ImGui) -> src/gui_2.py
GUI is a pure view; the registry/data classes are ops; commands.py owns
the registry because commands.py is where @registry.register decorators live.
gui_2.render_palette_modal imports Command from commands.py to type its
parameters.
Also fixes Phase 1.1 (bg_shader) per architecture feedback:
BackgroundShader no longer owns 'enabled' state - the GUI is pure view.
State is now owned by AppController.bg_shader_enabled (read on load from
config, written from gui_2 checkbox via app's __setattr__ delegation).
Tests:
- tests/test_command_palette.py: imports from src.commands (was src.command_palette)
- tests/test_commands_no_top_level_command_palette.py: rewritten for the
new architecture (eager registry in commands.py; render in gui_2; no
circular import between commands.py and gui_2)
This commit is contained in:
@@ -1,26 +1,22 @@
|
||||
"""Tests that src/commands.py has NO top-level src.command_palette import.
|
||||
"""Tests for the post-Phase 1.3 architecture.
|
||||
|
||||
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.
|
||||
Per module_taxonomy_refactor_20260627 Phase 1.3, src/command_palette.py was
|
||||
deleted and its content split by responsibility:
|
||||
- Command / ScoredCommand / CommandRegistry / fuzzy_match (data/ops) -> src/commands.py
|
||||
- render_palette_modal (view) -> src/gui_2.py
|
||||
|
||||
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).
|
||||
src/commands.py is a thin data module and can be imported eagerly (no
|
||||
require_warmed lazy load is needed; the original lazy-load pattern in
|
||||
startup_speedup_20260606 was specifically to defer the heavy src/command_palette
|
||||
which pulled in imgui at module load).
|
||||
|
||||
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
|
||||
These tests run in fresh subprocesses to ensure no warmup state leaks from
|
||||
the test runner. We assert:
|
||||
- src/commands imports cleanly and exposes Command + CommandRegistry
|
||||
- src/gui_2 exposes render_palette_modal
|
||||
- src/commands does NOT import gui_2 at module level (avoids circular)
|
||||
- The static audit detects no top-level command_palette import (since the
|
||||
module no longer exists)
|
||||
"""
|
||||
|
||||
import subprocess
|
||||
@@ -42,77 +38,63 @@ def _run_in_subprocess(snippet: str) -> subprocess.CompletedProcess:
|
||||
)
|
||||
|
||||
|
||||
def test_commands_does_not_import_command_palette_at_module_level() -> None:
|
||||
def test_commands_exposes_command_and_registry() -> 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)
|
||||
from src.commands import Command, CommandRegistry, fuzzy_match, ScoredCommand
|
||||
r = CommandRegistry()
|
||||
def my_cmd(app): pass
|
||||
r.register(my_cmd)
|
||||
print(len(r.all()))
|
||||
print(r.all()[0].id)
|
||||
""")
|
||||
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]}"
|
||||
assert lines[0] == "1"
|
||||
assert lines[1] == "my_cmd"
|
||||
|
||||
|
||||
def test_commands_register_decorator_is_lazy() -> None:
|
||||
"""The @registry.register decorator should NOT trigger command_palette import at module load."""
|
||||
def test_gui_2_exposes_render_palette_modal() -> None:
|
||||
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))
|
||||
from src.gui_2 import render_palette_modal
|
||||
print(callable(render_palette_modal))
|
||||
""")
|
||||
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]}"
|
||||
assert res.stdout.strip() == "True"
|
||||
|
||||
|
||||
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.
|
||||
def test_commands_does_not_import_gui_2_at_module_level() -> None:
|
||||
"""src/commands is imported by src/commands registration sites and by
|
||||
gui_2.render_palette_modal. To avoid a circular import, commands.py must
|
||||
NOT import gui_2 at the top of the file. (TYPE_CHECKING imports are
|
||||
allowed because they don't execute at runtime.)
|
||||
"""
|
||||
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']
|
||||
commands_path = Path('src') / 'commands.py'
|
||||
source = commands_path.read_text(encoding='utf-8')
|
||||
tree = ast.parse(source)
|
||||
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)
|
||||
if isinstance(node, (ast.Import, ast.ImportFrom)):
|
||||
mod = getattr(node, 'module', None) or (node.names[0].name if node.names else '')
|
||||
if mod and ('gui_2' in mod or mod.endswith('gui_2')):
|
||||
print('VIOLATION:', mod)
|
||||
print('OK')
|
||||
""")
|
||||
assert res.returncode == 0, f"stderr: {res.stderr}"
|
||||
assert "OK" in res.stdout
|
||||
assert "VIOLATION" not in res.stdout
|
||||
assert "OK" in res.stdout
|
||||
|
||||
|
||||
def test_command_palette_module_no_longer_exists() -> None:
|
||||
"""src/command_palette.py was deleted in Phase 1.3; this is a regression guard."""
|
||||
res = _run_in_subprocess("""
|
||||
import importlib
|
||||
try:
|
||||
importlib.import_module('src.command_palette')
|
||||
print('EXISTS')
|
||||
except ModuleNotFoundError:
|
||||
print('NOT_FOUND')
|
||||
""")
|
||||
assert res.returncode == 0, f"stderr: {res.stderr}"
|
||||
assert res.stdout.strip() == "NOT_FOUND"
|
||||
Reference in New Issue
Block a user