feat(llm): extract BaseVertexAILLM around a model-construction hook - #572
Draft
matteomedioli wants to merge 10 commits into
Draft
feat(llm): extract BaseVertexAILLM around a model-construction hook#572matteomedioli wants to merge 10 commits into
matteomedioli wants to merge 10 commits into
Conversation
matteomedioli
marked this pull request as draft
July 17, 2026 11:41
This was referenced Jul 17, 2026
matteomedioli
force-pushed
the
matteo/split-http-client-helper
branch
from
July 17, 2026 15:02
0b4426b to
4baa924
Compare
matteomedioli
force-pushed
the
matteo/base-vertexai-llm
branch
from
July 17, 2026 15:04
5df6253 to
e75b657
Compare
…ync Anthropic clients Route httpx.Client only to the sync anthropic.Anthropic constructor and httpx.AsyncClient only to the async anthropic.AsyncAnthropic constructor, mirroring BaseOpenAILLM's existing param-splitting pattern, so passing a sync-only or async-only http_client no longer collides across both clients.
… type Mirror BaseOpenAILLM's existing behavior: when http_client is provided but is neither an httpx.Client nor an httpx.AsyncClient instance, emit a warning and construct both clients with the default (no custom) http_client instead of raising.
…tests Add unit tests verifying httpx.Client reaches only the sync Anthropic client, httpx.AsyncClient reaches only the async client, and an invalid http_client type warns and falls back to defaults for both clients. Also note the stale-venv anthropic version gotcha in AGENTS.md.
Documents the AnthropicLLM sync/async http_client kwargs collision bug fixed in tasks 001-002, matching the repo's existing changelog style.
Keep the PR scoped to the http_client routing fix.
matteomedioli
force-pushed
the
matteo/split-http-client-helper
branch
from
July 17, 2026 15:07
4baa924 to
2b6a1f6
Compare
matteomedioli
force-pushed
the
matteo/base-vertexai-llm
branch
from
July 17, 2026 15:07
e75b657 to
7575cd1
Compare
AnthropicLLM, OpenAILLM, and AzureOpenAILLM each carried an identical ~10-line dance to route an httpx.Client/httpx.AsyncClient http_client kwarg to the matching SDK client, warning and dropping it otherwise. Factored into one split_http_client_kwargs helper in llm/utils.py, used by all three, so any future subclass needing a custom endpoint doesn't need a fourth/fifth copy of the same logic. Pure refactor, no behavior change.
… helper scope stacklevel=3 skips the LLM constructor frame that calls the helper, so the warning points at the user's constructor call site as it did when the logic was inline. Changelog no longer labels the helper 'internal': custom subclasses constructing their own SDK clients are expected to call it.
VertexAILLM has no persistent per-instance SDK client (unlike Anthropic/ OpenAI/Gemini) -- it relies on a global vertexai.init(...) plus a fresh GenerativeModel built per call. So instead of a client-construction hook, BaseVertexAILLM declares _get_model(...) as its one abstract method; VertexAILLM implements it exactly as before, and everything else (message building, generation-config/schema handling, response parsing) moves to the base class unchanged. BaseVertexAILLM is exported from neo4j_graphrag.llm as a documented extension point, mirroring BaseAnthropicLLM/BaseOpenAILLM/BaseGeminiLLM but with the mechanism this provider's SDK actually supports.
… to end A minimal subclass overriding only _get_model now proves its custom model is the one invoke() calls, with message building and response parsing inherited — the contract exported to custom subclasses.
Bound methods typed as Callable expose no __func__ under strict mypy (attr-defined, x4) — this fails CI's 'mypy .' gate. The end-to-end minimal-subclass test already covers the inheritance contract these asserts approximated.
matteomedioli
force-pushed
the
matteo/split-http-client-helper
branch
from
July 17, 2026 15:21
2b6a1f6 to
5a6541c
Compare
matteomedioli
force-pushed
the
matteo/base-vertexai-llm
branch
from
July 17, 2026 15:21
7575cd1 to
0fa27e8
Compare
matteomedioli
force-pushed
the
matteo/split-http-client-helper
branch
from
July 22, 2026 11:42
5a6541c to
a819760
Compare
matteomedioli
force-pushed
the
matteo/base-gemini-llm
branch
from
July 23, 2026 12:24
f943b3e to
5ca3ff9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack
Merge order: #565 → #567 → #566 → #571 → #572. Each PR builds on the previous one (this branch is stacked on
matteo/base-gemini-llm). Until its base merges, GitHub may show parent commits in the diff — review only the commits unique to this branch.Description
Same goal as the other base classes — subclass instead of fork — but VertexAI needs a different shape. There is no client object to construct: the SDK works through a global
vertexai.init(...)call plus a freshGenerativeModelcreated for every request.So instead of "the subclass creates the clients",
BaseVertexAILLMhas one abstract method,_get_model(), which returns the model used for a call. That is the only thing a subclass implements — pointing at a different project, region, or endpoint is a few lines. Everything else (message building, config and schema handling, response parsing) is inherited.VertexAILLMimplements_get_model()exactly as it did before the extraction — no behavior change.BaseVertexAILLMis exported fromneo4j_graphrag.llm; thellm.rstextensibility page now has a Vertex AI section explaining why this hook differs from the Anthropic/OpenAI/Gemini client-injection shape (nohttp_client/base_url— endpoint config is global viavertexai.init), with a worked subclass example.api.rstcross-references it.GeminiLLM(google-genai SDK), positioningVertexAILLMas legacy support — resolving the maintainer question below in the docs._get_model, and a minimal subclass overriding only_get_modelrunsinvoke()end to end with its custom model.Question for maintainers: Google is retiring the
vertexai.generative_modelsSDK in favor of google-genai — whichGeminiLLMalready uses, and which can also reach Vertex-hosted models. This PR still makes sense (it cleanly factors what exists); the docs now recommendGeminiLLMfor new work. Shout if you'd rather drop that note or phrase it differently.Type of Change
Complexity
Complexity: Low
How Has This Been Tested?
Checklist