fix(ai_client): use parent package lookup to fix google.genai circular import
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).
This commit is contained in:
+5
-14
@@ -47,22 +47,13 @@ def _require_warmed(name: str) -> Any:
|
||||
import cost is paid during startup on a bg thread, NOT on the first
|
||||
user-triggered AI call.
|
||||
|
||||
For dotted names (e.g. 'google.genai.types'), the parent packages
|
||||
are pre-imported first. This avoids circular-import errors in libraries
|
||||
like google-genai where importing a sub-module triggers a chain that
|
||||
re-enters the same sub-module (e.g. google.genai._api_client does
|
||||
'from .types import HttpOptions' during its own initialization).
|
||||
Without parent pre-import, that relative import would see a
|
||||
'partially initialized' google.genai.types in sys.modules and raise
|
||||
ImportError.
|
||||
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
|
||||
if "." in name:
|
||||
parts = name.split(".")
|
||||
for i in range(1, len(parts)):
|
||||
parent = ".".join(parts[:i])
|
||||
if parent not in sys.modules:
|
||||
importlib.import_module(parent)
|
||||
return importlib.import_module(name)
|
||||
|
||||
Reference in New Issue
Block a user