feat(llm): extract BaseGeminiLLM, add base_url support - #571
Merged
Conversation
matteomedioli
marked this pull request as draft
July 17, 2026 11:39
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-gemini-llm
branch
from
July 17, 2026 15:03
4183734 to
1622fed
Compare
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-gemini-llm
branch
from
July 17, 2026 15:07
1622fed to
6510347
Compare
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-gemini-llm
branch
from
July 17, 2026 15:21
6510347 to
44e0906
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 22, 2026 16:10
44e0906 to
ebfbf3b
Compare
matteomedioli
marked this pull request as ready for review
July 22, 2026 16:14
stellasia
reviewed
Jul 23, 2026
| raise LLMGenerationError(f"Error calling GeminiLLM with tools: {e}") from e | ||
|
|
||
| async def aclose(self) -> None: | ||
| self.client.close() |
Contributor
There was a problem hiding this comment.
Weird that we need both. Do they also have both a sync and an async connection internally?
Contributor
Author
There was a problem hiding this comment.
Seems so:
-
Client.close() and AsyncClient.aclose()
https://github.com/googleapis/python-genai/blob/v1.62.0/google/genai/client.py -
BaseApiClient.close() / aclose():
https://github.com/googleapis/python-genai/blob/v1.62.0/google/genai/_api_client.py
I run a quick minimal test:
import asyncio
from google import genai
async def main() -> None:
client = genai.Client(api_key="fake")
api = client._api_client
client.close()
print("after close(): sync closed:", api._httpx_client.is_closed,
"| async closed:", api._async_httpx_client.is_closed) # True | False
await client.aio.aclose()
print("after aio.aclose(): sync closed:", api._httpx_client.is_closed,
"| async closed:", api._async_httpx_client.is_closed) # True | True
asyncio.run(main())
stellasia
approved these changes
Jul 23, 2026
…sponse-parsing logic GeminiLLM now subclasses BaseGeminiLLM and is responsible only for constructing the genai.Client; everything else (get_messages, get_messages_v2, config/schema building, tool handling, response parsing) moves to the base class unchanged. BaseGeminiLLM is exported from neo4j_graphrag.llm as a documented extension point, mirroring BaseAnthropicLLM/BaseOpenAILLM. Also sets supports_structured_output = True on the base class: GeminiLLM already implements structured output via response_schema/response_mime_type but never declared the capability flag, so callers had to patch it in manually.
Review follow-ups: - Drop the supports_structured_output=True flag flip: it changes SimpleKGPipeline/extractor default behavior for existing GeminiLLM users and belongs in its own PR with a (breaking)-labeled changelog entry, mirroring how the AnthropicLLM flag change was documented. - Replace method-identity assertions (implementation-shape tests) with a minimal BaseGeminiLLM subclass test that exercises invoke() end to end — the actual exported extension contract.
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
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 (last two in either order). Each PR builds on the previous one. Until its base merges, GitHub may show parent commits in the diff — review only the commits unique to this branch.
Description
Same split as #566, applied to Gemini.
BaseGeminiLLMholds all the shared logic: message building, config and schema building, response parsing, tool handling.GeminiLLMbecomes a thin subclass whose only job is creating thegenai.Client.BaseGeminiLLMis exported fromneo4j_graphrag.llmas a supported extension point. A subclass reaching a custom endpoint just builds its own client — everything else is inherited. A test proves the contract: a minimal subclass that only assigns a client runsinvoke()end to end.base_urlparameter onGeminiLLM, mirroring feat(llm): extract BaseAnthropicLLM, add base_url, export extension points #566. Thegoogle-genaiSDK has no top-levelbase_urlargument, so the value is applied throughhttp_options; whenhttp_optionsis also passed (dict ortypes.HttpOptions), only itsbase_urlfield is overridden. All three merge paths are tested, plus the negative path (nobase_url→ nothing passed).aclose()now closes thegenai.Client(sync andaiosurfaces), sowith GeminiLLM(...)/async withactually release resources like the other providers.llm.rstextensibility page extended with a Gemini section (single-client model,http_optionsinstead ofhttp_client),api.rstcross-references,base_urlexample added.Behavior notes:
base_urlandacloseare small additive features on top.supports_structured_outputflag for Gemini. That silently changes pipeline defaults for existing users, so it moved to its own PR (feat(llm): GeminiLLM declares supports_structured_output = True #573) with a proper breaking-change note.GeminiLLM's parent class changes from the two interface classes toLLMBase, which extends both — every existingisinstancecheck still passes, no deprecation warning fires, and the class gains working context-manager support (with GeminiLLM(...) as llm:) like the other providers.Type of Change
Complexity
Complexity: Low
How Has This Been Tested?
Checklist