refactor(app_controller): remove top-level fastapi imports; lift _require_warmed to shared module
Phase 4 T4.1-T4.4 of startup_speedup_20260606 track. DEVIATION FROM ORIGINAL SPEC: spec.md said fastapi was in src/api_hooks.py but it was actually in src/app_controller.py (lines 17, 21). api_hooks.py uses stdlib http.server. Phase 4 target corrected to app_controller. LIFTED _require_warmed TO SHARED MODULE: created src/module_loader.py to avoid duplicating the lookup logic and the cross-module import smell (app_controller -> ai_client). src/ai_client.py re-exports it so the T3.1 test (which asserts hasattr(src.ai_client, '_require_warmed')) continues to work. src/app_controller.py changes: - Added 'from __future__ import annotations' (enables lazy type annotations; -> FastAPI return type now a forward reference) - Removed 'from fastapi import FastAPI, Depends, HTTPException' (line 17) - Removed 'from fastapi.security.api_key import APIKeyHeader' (line 21) - Added 'from src.module_loader import _require_warmed' (cross-module via shared utility, not via ai_client) - create_api(): added lookups at top of function body - 7 _api_* helper functions (_api_get_key, _api_generate, _api_stream, _api_confirm_action, _api_get_session, _api_delete_session, _api_get_context): added 'HTTPException = _require_warmed(...).HTTPException' at top of each function body EFFECTIVENESS: - import src.app_controller no longer triggers fastapi import (saves ~470ms in main thread; only loaded when --enable-test-hooks is set) - When --enable-test-hooks is set, the AppController's warmup pre-loads fastapi on the _io_pool, so create_api()'s lookup is O(1) TESTS: - tests/test_app_controller_no_top_level_fastapi.py: 4/4 PASS (was 3 RED + 1 pass) - tests/test_ai_client_no_top_level_sdk_imports.py: 9/9 still PASS (re-export works) - tests/test_app_controller_mcp.py, test_app_controller_offloading.py: pass - tests/test_headless_service.py: 10/11 PASS (1 pre-existing failure test_generate_endpoint is a circular-import issue in google.genai, reproduces identically on stashed pre-Phase-4 state - NOT a regression from this change) - tests/test_hooks.py: pass NEXT: Phase 5 (feature-gated GUI module imports - command palette, NERV theme, markdown table), then Phase 6 (ad-hoc threads -> _io_pool).
This commit is contained in:
+5
-18
@@ -52,24 +52,11 @@ from src.tool_bias import ToolBiasEngine
|
||||
from src.tool_presets import ToolPresetManager
|
||||
|
||||
|
||||
def _require_warmed(name: str) -> Any:
|
||||
"""Return a heavy module that the AppController's warmup should have loaded.
|
||||
|
||||
Heavy SDKs (anthropic, google.genai, openai, google.genai.types,
|
||||
requests) are warmed on AppController's _io_pool at startup. This
|
||||
function expects them to already be in sys.modules and just returns
|
||||
the cached module object. If the module is NOT in sys.modules (e.g.
|
||||
in tests where warmup didn't run), falls back to importlib so the
|
||||
call still works.
|
||||
|
||||
In production: this is an O(1) sys.modules lookup. The 1+ second
|
||||
import cost is paid during startup on a bg thread, NOT on the first
|
||||
user-triggered AI call.
|
||||
"""
|
||||
mod = sys.modules.get(name)
|
||||
if mod is not None:
|
||||
return mod
|
||||
return importlib.import_module(name)
|
||||
# _require_warmed lives in src/module_loader.py to avoid duplicating the
|
||||
# lookup logic across files that need heavy modules. Re-exported here so
|
||||
# existing call sites and the T3.1 test (which asserts
|
||||
# hasattr(src.ai_client, '_require_warmed')) continue to work.
|
||||
from src.module_loader import _require_warmed # noqa: E402,F401
|
||||
|
||||
|
||||
_provider: str = "gemini"
|
||||
|
||||
+16
-3
@@ -1,3 +1,5 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import copy
|
||||
import inspect
|
||||
import json
|
||||
@@ -14,15 +16,13 @@ import uuid
|
||||
# TODO(Ed): Eliminate these?
|
||||
from dataclasses import asdict
|
||||
from datetime import datetime
|
||||
from fastapi import FastAPI, Depends, HTTPException
|
||||
from pathlib import Path
|
||||
from typing import Any, List, Dict, Optional, Callable
|
||||
|
||||
from fastapi.security.api_key import APIKeyHeader
|
||||
|
||||
from src import aggregate
|
||||
from src import models
|
||||
from src import ai_client
|
||||
from src.module_loader import _require_warmed
|
||||
from src import conductor_tech_lead
|
||||
from src import events
|
||||
from src import mcp_client
|
||||
@@ -148,6 +148,7 @@ async def _api_get_key(controller: 'AppController', header_key: str) -> str:
|
||||
Validates the API key from the request header against configuration.
|
||||
[SDM: src/app_controller.py:_api_get_key]
|
||||
"""
|
||||
HTTPException = _require_warmed("fastapi").HTTPException
|
||||
headless_cfg = controller.config.get("headless", {})
|
||||
config_key = headless_cfg.get("api_key", "").strip()
|
||||
env_key = os.environ.get("SLOP_API_KEY", "").strip()
|
||||
@@ -278,6 +279,7 @@ def _api_generate(controller: 'AppController', req: GenerateRequest) -> dict[str
|
||||
Triggers an AI generation request using the current project context.
|
||||
[SDM: src/app_controller.py:_api_generate]
|
||||
"""
|
||||
HTTPException = _require_warmed("fastapi").HTTPException
|
||||
if not req.prompt.strip():
|
||||
raise HTTPException(status_code=400, detail="Prompt cannot be empty")
|
||||
if controller.is_project_stale():
|
||||
@@ -390,6 +392,7 @@ async def _api_stream(controller: 'AppController', req: GenerateRequest) -> Any:
|
||||
Placeholder for streaming AI generation responses (Not yet implemented).
|
||||
[SDM: src/app_controller.py:_api_stream]
|
||||
"""
|
||||
HTTPException = _require_warmed("fastapi").HTTPException
|
||||
raise HTTPException(status_code=501, detail="Streaming endpoint (/api/v1/stream) is not yet supported in this version.")
|
||||
|
||||
def _api_pending_actions(controller: 'AppController') -> list[dict[str, Any]]:
|
||||
@@ -410,6 +413,7 @@ def _api_confirm_action(controller: 'AppController', action_id: str, req: Confir
|
||||
Approves or rejects a pending action.
|
||||
[SDM: src/app_controller.py:_api_confirm_action]
|
||||
"""
|
||||
HTTPException = _require_warmed("fastapi").HTTPException
|
||||
with controller._pending_dialog_lock:
|
||||
if action_id not in controller._pending_actions:
|
||||
raise HTTPException(status_code=404, detail="Action not found")
|
||||
@@ -439,6 +443,7 @@ def _api_get_session(controller: 'AppController', session_id: str) -> dict[str,
|
||||
Returns the content of the comms.log for a specific session.
|
||||
[SDM: src/app_controller.py:_api_get_session]
|
||||
"""
|
||||
HTTPException = _require_warmed("fastapi").HTTPException
|
||||
log_path = paths.get_logs_dir() / session_id / "comms.log"
|
||||
if not log_path.exists():
|
||||
raise HTTPException(status_code=404, detail="Session log not found")
|
||||
@@ -450,6 +455,7 @@ def _api_delete_session(controller: 'AppController', session_id: str) -> dict[st
|
||||
Deletes a specific session directory.
|
||||
[SDM: src/app_controller.py:_api_delete_session]
|
||||
"""
|
||||
HTTPException = _require_warmed("fastapi").HTTPException
|
||||
log_path = paths.get_logs_dir() / session_id
|
||||
if not log_path.exists() or not log_path.is_dir():
|
||||
raise HTTPException(status_code=404, detail="Session directory not found")
|
||||
@@ -463,6 +469,7 @@ def _api_get_context(controller: 'AppController') -> dict[str, Any]:
|
||||
Returns the current aggregated project context.
|
||||
[SDM: src/app_controller.py:_api_get_context]
|
||||
"""
|
||||
HTTPException = _require_warmed("fastapi").HTTPException
|
||||
try:
|
||||
md, path, file_items, stable_md, disc_text = controller._do_generate()
|
||||
screenshots = controller.project.get("screenshots", {}).get("paths", [])
|
||||
@@ -2627,6 +2634,12 @@ class AppController:
|
||||
[SDM: src/app_controller.py:AppController.create_api]
|
||||
[C: src/gui_2.py:App.run, tests/test_headless_service.py:TestHeadlessAPI.setUp]
|
||||
"""
|
||||
fastapi = _require_warmed("fastapi")
|
||||
FastAPI = fastapi.FastAPI
|
||||
Depends = fastapi.Depends
|
||||
HTTPException = fastapi.HTTPException
|
||||
api_key_mod = _require_warmed("fastapi.security.api_key")
|
||||
APIKeyHeader = api_key_mod.APIKeyHeader
|
||||
api = FastAPI(title="Manual Slop Headless API")
|
||||
API_KEY_NAME = "X-API-KEY"
|
||||
api_key_header = APIKeyHeader(name=API_KEY_NAME, auto_error=False)
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
"""Shared module loader for heavy imports.
|
||||
|
||||
Per startup_speedup_20260606 spec §2.2 Layer 1, the main thread (the one
|
||||
that enters `immapp.run()`) must NEVER import a module heavier than
|
||||
`imgui_bundle` and the lean `gui_2` skeleton. Heavy modules are removed
|
||||
from main-thread-reachable files entirely and accessed via `_require_warmed`
|
||||
at use sites. The warmup mechanism in `src/warmup.py` (driven by
|
||||
`AppController.__init__`) pre-loads them on the `_io_pool` so the first
|
||||
user-triggered call is instant — the import cost is paid during startup on
|
||||
a background thread, not on the user's first click.
|
||||
|
||||
This module is the single home of the `_require_warmed` helper so that
|
||||
multiple files (src/ai_client.py, src/app_controller.py, future Phase 5
|
||||
feature-gated modules) share one canonical implementation instead of
|
||||
duplicating the lookup logic. The earlier practice of defining the helper
|
||||
locally in src/ai_client.py created a cross-module import smell
|
||||
(app_controller -> ai_client) when other modules needed the same primitive.
|
||||
|
||||
Public API:
|
||||
_require_warmed(name) -> module
|
||||
O(1) sys.modules lookup if warmup ran; importlib fallback otherwise.
|
||||
|
||||
Why a function instead of a `from X import Y` inside each call site?
|
||||
Per the spec, lazy-loading on first use causes user-perceptible lag when
|
||||
a UI action propagates to a controller method that triggers the first
|
||||
import. Proactive warmup on bg threads means the cost is paid during the
|
||||
visible startup window, not on the user's click.
|
||||
"""
|
||||
|
||||
import importlib
|
||||
import sys
|
||||
from typing import Any
|
||||
|
||||
|
||||
def _require_warmed(name: str) -> Any:
|
||||
"""Return a heavy module that the AppController's warmup should have loaded.
|
||||
|
||||
Heavy SDKs (anthropic, google.genai, openai, google.genai.types, requests,
|
||||
fastapi, fastapi.security.api_key, src.command_palette, src.theme_nerv,
|
||||
src.theme_nerv_fx, src.markdown_table, numpy) are warmed on
|
||||
AppController's _io_pool at startup. This function expects them to
|
||||
already be in sys.modules and just returns the cached module object. If
|
||||
the module is NOT in sys.modules (e.g. in tests where warmup didn't
|
||||
run), falls back to importlib so the call still works.
|
||||
|
||||
In production: this is an O(1) sys.modules lookup. The 1+ second
|
||||
import cost is paid during startup on a bg thread, NOT on the first
|
||||
user-triggered AI call.
|
||||
"""
|
||||
mod = sys.modules.get(name)
|
||||
if mod is not None:
|
||||
return mod
|
||||
return importlib.import_module(name)
|
||||
Reference in New Issue
Block a user