The matrix has v2 fields (reasoning, web_search, x_search)
populated for the old vendors (minimax-M2.5/M2.7, grok-*),
but the send functions didn't consult them. This commit
makes the code path actually USE the matrix:
_send_minimax: gate reasoning_extractor on caps.reasoning
(was unconditional; now skipped for non-reasoning models
to avoid useless getattr calls)
_send_grok: populate OpenAICompatibleRequest.extra_body with
search_parameters when caps.web_search or caps.x_search is
True. caps.web_search -> {mode: auto}; caps.x_search ->
{sources: [{type: x}]} per the xAI Live Search spec
OpenAICompatibleRequest: added extra_body field. Wired
through send_openai_compatible (passed as extra_body kwarg
to client.chat.completions.create).
Also fixed 2 latent bugs in _send_minimax surfaced by the
new tests: the function was missing 'tools' variable
(NameError) and 'stream_callback' parameter. These are
pre-existing bugs masked by mock-based tests that don't
exercise the actual call path.
Also cancelled t5_6/7/8 (the invented 'deferred tool-loop
conversion' work). The 3 vendors (anthropic, gemini,
deepseek) use vendor-specific call paths. Their inline
loops are NOT defects. The '3-5 days' / '1-2 weeks'
estimates were made up by the agent. The audit script's
DEFERRED_VENDORS exclusion is permanent.
Tests:
- 2 new grok tests: web_search and x_search populate
extra_body correctly
- 2 new minimax tests: reasoning_extractor used/omitted
based on caps.reasoning
- 122/122 vendor+tool+provider+import-isolation tests pass
(no regressions; +4 new tests this commit)
- 3 audit scripts pass
When _llama_base_url is localhost/127.0.0.1, _send_llama now
calls _send_llama_native (the native /api/chat adapter)
instead of the OpenAI-compat path. The native adapter
supports Ollama's vendor-specific fields: think, images,
thinking.
Functions added (in src/ai_client.py, per the naming
convention HARD RULE on no new src/*.py files):
ollama_chat(model, messages, *, think='low', images=None,
tools=None, base_url=OLLAMA_DEFAULT_BASE_URL)
-> dict[str, Any]
_send_llama_native(md_content, user_message, base_dir,
file_items=None, discussion_history='',
stream=False, ...callbacks) -> str
OLLAMA_DEFAULT_BASE_URL: str = 'http://localhost:11434'
Implementation notes:
- requests loaded via _require_warmed('requests') (local
scope; preserves startup_speedup_20260606 invariant that
heavy SDKs are warmed on _io_pool, not imported at module
level)
- _send_llama dispatches based on 'localhost' in
_llama_base_url (same check already used by
_get_llama_cost_tracking at line 2500)
- Removed orphan def stub at the old _send_llama body (the
dead 'def _build_llama_request' that was overwritten by
the real one — a known session issue with stale set_file_slice
edits)
- Native adapter appends the 'thinking' field to history so
subsequent rounds preserve the reasoning chain
Tests:
- 7 new tests in tests/test_llama_ollama_native.py:
* ollama_chat hits /api/chat (not /v1/chat/completions)
* ollama_chat includes 'think' param in payload
* ollama_chat includes 'images' in payload
* _send_llama_native wraps ollama_chat
* _send_llama_native preserves 'thinking' field
* _send_llama routes localhost to native (no openai client)
* _send_llama keeps openai path for non-local (no POST)
- Updated test_send_llama_ollama_backend in test_llama_provider.py
to mock the native path (was: mocked openai-compat; now:
mocked requests.post)
- 103/103 vendor+tool+provider+import-isolation tests pass
(no regressions; +7 new tests this commit)
- 4 audit scripts pass
Phase 2 tasks 2.1 + 2.2 + 2.3a of the follow-up track.
PROVIDERS now lives in src/ai_client.py:56 (the canonical home for
AI-client-related constants per the HARD RULE on src/ files). The
list includes all 8 vendors: gemini, anthropic, gemini_cli,
deepseek, minimax, qwen, grok, llama.
Backward compat: src/models.py:PROVIDERS is exposed via a module-
level __getattr__ (PEP 562) that lazy-imports from src.ai_client.
The lazy approach was needed because src.ai_client imports
ToolPreset/BiasProfile/Tool from src.models at line 50, so a
top-level 'from src.ai_client import PROVIDERS' in models.py
would deadlock. Adding a branch to the existing __getattr__
in models.py (which also handles pydantic class factories) is
the surgical fix.
tests/test_provider_curation.py was stale (expected 5 providers
from before Qwen/Grok/Llama were added). Updated to 8.
New test: tests/test_providers_source_of_truth.py asserts:
- src.ai_client.PROVIDERS exists and matches the 8-provider list
- src.models.PROVIDERS still works (re-export)
- Both modules reference the SAME object (no drift)
Green confirmed: 4 provider tests pass.
The follow-up track's tool-loop refactor moved
'from src.openai_compatible import send_openai_compatible,
OpenAICompatibleRequest, NormalizedResponse' to MODULE level
in src/ai_client.py. This violates the startup_speedup_20260606
invariant: heavy SDKs must not be loaded at module level because
ai_client.py is on the main thread's import chain.
src/openai_compatible.py line 5 does 'from openai import
OpenAIError, ...', so any import from it triggers the openai SDK
to load. test_ai_client_does_not_import_openai_at_module_level
guards this invariant and was failing.
Fix: move the imports back to local scope inside the function
bodies that need them:
- _default_send closure inside run_with_tool_loop
(imports send_openai_compatible)
- _send_grok (imports OpenAICompatibleRequest)
- _send_minimax (imports OpenAICompatibleRequest)
- _send_llama (imports OpenAICompatibleRequest)
- _send_gemini_cli (imports OpenAICompatibleRequest + NormalizedResponse)
Test patches: tests that previously patched
'src.ai_client.send_openai_compatible' now patch
'src.openai_compatible.send_openai_compatible' (the actual
import source). _execute_tool_calls_concurrently patches
unchanged (it's defined in src/ai_client.py itself).
Green confirmed: 62 vendor + tool + import-isolation tests
pass. 0 regressions.
Task 1.7 of the follow-up track. Extends run_with_tool_loop with
two optional parameters that let vendored call paths share the
shared loop + history + dispatch without forcing them through
send_openai_compatible:
- send_func: Callable[[int], NormalizedResponse] - vendor's own
API call (default = send_openai_compatible if not provided;
fully backward compatible)
- on_pre_dispatch: Callable[[int, list[dict]], list[dict]] -
per-vendor hook to mutate the tool-call list before dispatch
AND to capture results for the next round (e.g. Gemini CLI
sets payload = tool_results_for_cli so the next send_func
call sends the tool results back to the CLI)
_refactor _send_gemini_cli to use the new parameters. The
inline for loop + tool dispatch + history append are all
delegated to the helper. The vendor's send_func closure
handles:
- adapter.send (the CLI subprocess call)
- resp_data parsing (text + tool_calls + usage + stderr)
- events.emit for request_start + response_received
- _append_comms for IN/OUT comms logging
- The 'txt + calls -> history_add' special case
The vendor's on_pre_dispatch closure handles:
- _execute_tool_calls_concurrently (re-invoked here because
the helper's call passes raw tool_calls but the vendor
needs to mutate payload AND log results)
- _reread_file_items + _build_file_diff_text (file diff
re-read at last tool result)
- MAX_ROUNDS system message
- _truncate_tool_output
- _MAX_TOOL_OUTPUT_BYTES budget warning
- Payload mutation for the next round
Green confirmed: 53 vendor + tool tests pass (14 Gemini CLI
+ 5 tool_loop core + 1 builder + 2 send_func + 6 MiniMax +
2 Grok + 7 Llama + 9 DeepSeek + 8 others). No regressions.
Task 1.6 of the follow-up track. _send_grok and _send_llama now
share the same tool-loop helper as the rest of the vendors.
Both functions add tool-calling support that they previously
lacked (parent Phase 3 shipped them as single-shot only). The
plan's Task 1.6 title says 'add missing loop' which matches
this scope. tool_choice='auto' if tools else 'auto' matches
the MiniMax pattern.
Qwen deferral: _send_qwen uses _dashscope_call (DashScope
native SDK), not send_openai_compatible. run_with_tool_loop
hard-codes send_openai_compatible. Wiring Qwen through the
helper requires either (a) switching Qwen to OpenAI-compat
mode, or (b) adding a Qwen-specific loop variant that uses
_dashscope_call. Both are non-trivial and out of scope for
Task 1.6. Tracked as a follow-up note in the state.toml.
Module-level imports added (same pattern as the previous
commits in this track): OpenAICompatibleRequest, get_capabilities
were imported locally inside the affected functions. Moved
to module-level so the test patches and helper signature can
reference them by symbol.
Green confirmed: 51 vendor + tool tests pass.
Task 1.3 of the follow-up track. _send_minimax now uses
run_with_tool_loop with a per-round request_builder callback
that re-reads _minimax_history under _minimax_history_lock.
The plan's Task 1.3 example builds the request once before the
loop. That would break MiniMax tool flows because the API
would not see the tool results appended to _minimax_history
on later rounds. The fix: extend run_with_tool_loop's 2nd arg
to accept Union[OpenAICompatibleRequest, Callable[[int],
OpenAICompatibleRequest]] (backward compatible; static-request
vendors pass a single request). MiniMax now passes a closure
that rebuilds messages from history each round.
Reasoning extraction: MiniMax exposes its chain-of-thought via
response.raw_response.choices[0].message.reasoning_details[0].
get('text'). Lifted to a _extract_minimax_reasoning callback
passed as reasoning_extractor=... (the new parameter added
in the previous commit).
Trim callback: wraps _trim_minimax_history so it can be called
from run_with_tool_loop after each tool-result append.
Green confirmed: 51 vendor + tool tests pass (6 MiniMax + 5
tool_loop core + 1 tool_loop builder + 39 others); the new
test_ai_client_tool_loop_builder.py locks in the per-round
builder contract.
Tasks 1.1 (red) + 1.2 (green) of the follow-up track. Adds a single
shared tool-call loop in src/ai_client.py that all 8 vendor entry
points (anthropic, gemini, gemini_cli, deepseek, minimax, qwen, grok,
llama) can call instead of maintaining their own inline loop.
Function shape:
- 1-space indentation (project standard)
- 60 lines (vs ~30 lines of inline loop body per vendor)
- Operates on src.openai_compatible.send_openai_compatible
(no local import — module-level import added for the same path
used by the 4 inline-loop vendors)
- 8 vendor-specific knobs: pre_tool_callback, qa_callback,
stream_callback, patch_callback, base_dir, vendor_name,
history_lock, history, trim_func, reasoning_extractor
- Threads the asyncio.get_running_loop / RuntimeError fallback
to handle the no-event-loop case (matches the existing
inline pattern from _send_minimax)
- Uses _execute_tool_calls_concurrently (the existing concurrent
dispatcher) — no new dispatch code
Deviations from plan/Task 1.1:
- The plan's test code patched src.tool_loop.send_openai_compatible
and the plan's Task 1.3 vendor wrapper imported 'from
src.tool_loop import run_with_tool_loop'. The plan predates the
AGENTS.md HARD RULE on src/<thing>.py files; per the follow-up
track's Naming Convention section, run_with_tool_loop lives IN
src/ai_client.py. Tests patch src.ai_client.send_openai_compatible
and the vendor wrapper imports 'from src.ai_client import
run_with_tool_loop' (next task).
- Added a reasoning_extractor: Callable[[Any], str] = None parameter
to support MiniMax's reasoning_content extraction. Without this
the helper would force MiniMax to lose its reasoning prefix.
Green confirmed: 50 vendor + tool tests pass; 4 audit scripts pass.
The previous refactor (commit 344a66fc) dropped the tool-call loop
in _send_minimax. The original function executed tool calls when the
response had tool_calls; the refactor was single-shot. This is a real
behavior regression (tools stop working) even though the existing
tests don't catch it.
Restore the tool loop:
- For each round (up to MAX_TOOL_ROUNDS + 2), call send_openai_compatible
with tools=_get_deepseek_tools() and tool_choice='auto'
- If response has tool_calls: dispatch each via
_execute_tool_calls_concurrently (handles both async context and
sync via run_coroutine_threadsafe / asyncio.run), append each
result to _minimax_history with role='tool' and tool_call_id
- If no tool_calls: return the response text (with thinking tags for
reasoning models)
- The lock is acquired/released per iteration to avoid holding it
during the API call (which can take seconds)
Preserved:
- 10-arg signature
- _minimax_history_lock (now acquired per iteration)
- _repair_minimax_history
- discussion_history handling
- System + context message wrapping
- Reasoning content extraction (response.raw_response.choices[0].message
.reasoning_details[0].get('text', ''))
- <thinking> tags wrap on the final response
Dropped (still):
- extra_body={reasoning_split: True} (not supported by send_openai_compatible;
would be a Phase 5 adapter addition if minimax-reasoner models need it)
New line count: 75 lines (vs 41 single-shot, vs 231 pre-refactor).
Net effect: 231 -> 75 = 68% reduction; tool loop preserved.
Verification: 38/38 tests pass (no regressions).
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).
Three test failures identified by the batched test suite, all rooted
in the Phase 3 lazy-import refactor of src/ai_client.py.
FIX 1: UnboundLocalError in _ensure_gemini_client
- _ensure_gemini_client had a latent bug: creds was assigned inside
`if _gemini_client is None:` but used on the next line. When the
client was already cached, the assignment was skipped and the next
line raised UnboundLocalError. Moved the Client() construction
inside the if block to match creds' scope.
- This affected test_ai_cache_tracking.py and (downstream)
test_gui_updates.py::test_telemetry_data_updates_correctly.
FIX 2: Phase 3 removed top-level `import requests` from ai_client.py.
- test_discussion_compression.py::test_discussion_compression_deepseek
did `patch("src.ai_client.requests.post", ...)` which no longer works.
- Updated the test to mock _require_warmed to return a fake requests
module with `.post()`, matching the new lazy-import pattern.
FIX 3: _require_warmed could not import dotted names like `google.genai.types`
- The google-genai library has a self-referential __init__.py that
does `from .client import Client` which transitively does
`from .types import HttpOptions`. Importing `google.genai.types`
FIRST (before the parent package is fully loaded) hit a "partially
initialized module" circular import.
- Enhanced _require_warmed to pre-import parent packages for dotted
names: walks `name.split(".")` and imports each parent (if not in
sys.modules) before the leaf import. O(n) extra imports per call
on first use; subsequent calls are O(1) sys.modules hit.
TESTS:
- test_ai_cache_tracking.py: 2/2 PASS
- test_discussion_compression.py: 4/4 PASS
- 29/29 PASS across the sampled test files that were failing
(test_subagent_summarization, test_tool_access_exclusion,
test_tier4_interceptor, test_gui2_mcp, test_gui_updates,
test_headless_service)
ARCHITECTURAL NOTE: The _require_warmed enhancement is a small
but important robustness fix. The google-genai library's
__init__.py chain is a known source of fragility; the parent-
pre-import pattern is the recommended workaround.
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).
Phase 3 T3.2 + T3.3 of startup_speedup_20260606 track.
The 5 heavy SDKs (anthropic, google.genai, openai, google.genai.types,
requests) are no longer imported at module level. Each function that
needs them now calls _require_warmed(name) to get the module from
sys.modules (populated by AppController's warmup on _io_pool).
This is the load-bearing wall of the Main Thread Purity Invariant:
heavy modules are never in the main thread's import chain.
run_discussion_compression now uses _require_warmed for both
google.genai.types (gemini branch) and requests (deepseek branch).
Tests/test_tier4_patch_generation.py adapted: the 2 tests that
mocked 'src.ai_client.types' (no longer a module-level attr)
now mock 'src.ai_client._require_warmed' (the new public mechanism).
T3.1 tests now pass (9/9). T3.3 breakage fixed.
All 25 ai_client + tier4 tests pass.
- Restore monolithic architecture in gui_2.py to fix test compatibility.
- Implement full-width horizontal expansion for Markdown tables in discussion entries.
- Re-implement layered role-based tints using draw_list channels.
- Standardize Text Viewer docking ID to '###Text_Viewer_Unified'.
- Fix MiniMax compression routing and base URL.
- Fully restore missing theme_2.py definitions.
- Restore monolithic architecture in gui_2.py to fix test breakages and circular imports.
- Update Text Viewer stable ID to '###Text_Viewer_Unified' to definitively fix docking conflicts.
- Refactor discussion entry renderer to force full-width horizontal expansion for Markdown.
- Fully restore theme_2.py definitions (palettes, fonts, scale) while retaining role-tint logic.
- Robustify ImGui ID stack in imgui_scopes.py to prevent access violations.
- Verify all fixes with the comprehensive unit and visual test suite.
- Modularize discussion entry rendering to src/discussion_entry_renderer.py to fix layout squashing.
- Fix MiniMax compression routing with robust case-insensitive check and synced base URL.
- Implement src/ui_shared.py to resolve circular imports and consolidate shared UI helpers.
- Finalize Structural File Editor integration and state unification.
- Correctly route 'minimax' provider in run_discussion_compression.
- Fix MiniMax base URL to api.minimax.io to match main sender.
- Refactor read-mode discussion entries to always use a scrollable child with auto-resize.
- Remove redundant text wrapping that caused Markdown tables to squash vertically.
- Clean up duplicate separators in discussion hub.
- Update _trim_minimax_history to drop dangling 'tool' messages if their parent 'assistant' message is removed.
- Fixes 'invalid params, tool call result does not follow tool call (2013)' error when token limit is hit.
- Display token metrics (input/output/cache) per response in Discussion Hub.
- Add total Discussion Token usage in the panel header.
- Implement 'Compress' feature to intelligently summarize and replace exhausted discussion histories using an AI subagent.
- Add _repair_minimax_history to close dangling tool calls from interrupted sessions.
- Add _trim_minimax_history to manage token limits and intelligently prune history.
- Integrate repair and trimming into _send_minimax loop.
- Resolves MiniMax error 2013 (tool call result does not follow tool call).
ed
r00tz