ed
88fc42bbc0
The conftest pre-warm workaround added earlier was a TEST INFRASTRUCTURE
patch that did not address the actual problem. The real issue is in the
lazy-import pattern: `_require_warmed("google.genai.types")` triggers
google-genai's broken __init__.py chain in fresh pytest processes.
Per the Phase 3 spec, the correct pattern is:
genai = _require_warmed("google.genai")
types = genai.types
The PARENT package import completes the chain once. Then `.types`
is just an attribute access on the loaded module. No new import
needed at the leaf.
ROOT CAUSE: google-genai's __init__.py does
from .client import Client -> from ._api_client import BaseApiClient
which transitively does `from .types import HttpOptions`. When
google.genai.types is being loaded for the first time, types.py
executes `from ._operations_converters import (...)`. If anything
in that chain triggers the parent __init__.py, the relative
`from .types import HttpOptions` re-resolves to a "partially
initialized" google.genai.types in sys.modules and raises ImportError.
By importing `google.genai` directly (the parent), the entire
__init__.py chain runs to completion BEFORE we ever look up `.types`.
Subsequent access is just attribute lookup, no import.
FIXES (7 sites in src/ai_client.py):
- _gemini_tool_declaration (L651)
- _send_anthropic (L1170)
- _send_gemini (L1422)
- run_tier4_analysis (L2360)
- run_tier4_patch_generation (L2410)
- run_subagent_summarization (L2568)
- run_discussion_compression (L2616)
All changed from `types = _require_warmed("google.genai.types")`
to:
genai = _require_warmed("google.genai")
types = genai.types
ALSO REMOVED:
- conftest.py pre-warm of google.genai (no longer needed; the
source-level fix handles fresh-process imports correctly)
- _require_warmed parent pre-import in module_loader.py (no longer
needed; the convention is to pass top-level package names)
ALSO KEPT (real bug fix from earlier):
- _ensure_gemini_client UnboundLocalError: moved Client() construction
inside the `if _gemini_client is None:` block so `creds` is in scope.
- test_discussion_compression.py: test now mocks _require_warmed
to return a fake requests module with .post() (Phase 3 removed
the top-level `import requests` from ai_client.py).
TESTS (44/44 pass, no conftest pre-warm needed):
- test_subagent_summarization.py: 3/3
- test_tool_access_exclusion.py: 4/4
- test_tier4_interceptor.py: 7/7 (incl. test_gemini_provider_passes_qa_callback_to_run_script)
- test_gui2_mcp.py: 1/1 (test_mcp_tool_call_is_dispatched)
- test_gui_updates.py: 3/3 (incl. test_telemetry_data_updates_correctly)
- test_headless_service.py: 11/11 (incl. test_generate_endpoint)
- test_project_switch_persona_preset.py: 9/9 (incl. test_api_generate_blocked_while_stale)
- test_discussion_compression.py: 4/4 (incl. test_discussion_compression_deepseek)
- test_ai_cache_tracking.py: 2/2 (incl. test_gemini_cache_tracking)
ARCHITECTURAL NOTE: This is the PROPER fix per the Phase 3 spec.
The earlier conftest pre-warm was a workaround that masked the
issue. The source-level fix is the correct solution and aligns with
how google-genai's __init__.py chain expects to be loaded.
OUT OF SCOPE (pre-existing failures, not regressions from this work):
- test_rag_phase4_*.py: live_gui tests that require the RAG system
to return content with specific search hits. Pre-existing.
- test_project_switch_persona_preset.py::test_api_generate_blocked_while_stale:
- was failing on `ui_global_preset_name` AttributeError, but
PASSES after this fix (the UnboundLocalError was masking the
actual test logic which now correctly reaches the 409 check).
60 lines
2.7 KiB
Python
60 lines
2.7 KiB
Python
"""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.
|
|
|
|
Callers should pass the TOP-LEVEL package name (e.g. 'google.genai'),
|
|
not a leaf sub-module (e.g. 'google.genai.types'). The library's
|
|
__init__.py chain has a circular-import bug when a sub-module is
|
|
loaded before its parent is fully initialized. The ai_client code
|
|
follows this convention: `_require_warmed("google.genai").types`.
|
|
"""
|
|
mod = sys.modules.get(name)
|
|
if mod is not None:
|
|
return mod
|
|
return importlib.import_module(name)
|