Skip to content

feat(llm): extract BaseGeminiLLM, add base_url support - #571

Merged
matteomedioli merged 4 commits into
mainfrom
matteo/base-gemini-llm
Jul 24, 2026
Merged

feat(llm): extract BaseGeminiLLM, add base_url support#571
matteomedioli merged 4 commits into
mainfrom
matteo/base-gemini-llm

Conversation

@matteomedioli

@matteomedioli matteomedioli commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

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.

  • New BaseGeminiLLM holds all the shared logic: message building, config and schema building, response parsing, tool handling. GeminiLLM becomes a thin subclass whose only job is creating the genai.Client.
  • BaseGeminiLLM is exported from neo4j_graphrag.llm as 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 runs invoke() end to end.
  • New explicit base_url parameter on GeminiLLM, mirroring feat(llm): extract BaseAnthropicLLM, add base_url, export extension points #566. The google-genai SDK has no top-level base_url argument, so the value is applied through http_options; when http_options is also passed (dict or types.HttpOptions), only its base_url field is overridden. All three merge paths are tested, plus the negative path (no base_url → nothing passed).
  • aclose() now closes the genai.Client (sync and aio surfaces), so with GeminiLLM(...) / async with actually release resources like the other providers.
  • Docs: llm.rst extensibility page extended with a Gemini section (single-client model, http_options instead of http_client), api.rst cross-references, base_url example added.

Behavior notes:

  • The extraction itself is behavior-neutral (commits 1–2); base_url and aclose are small additive features on top.
  • An earlier version of this PR also enabled the supports_structured_output flag 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 to LLMBase, which extends both — every existing isinstance check 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

  • New feature
  • Bug fix
  • Breaking change
  • Documentation update
  • Project configuration change

Complexity

Complexity: Low

How Has This Been Tested?

  • Unit tests
  • E2E tests
  • Manual tests

Checklist

  • Documentation has been updated
  • Unit tests have been updated
  • Examples have been updated
  • CLA (https://neo4j.com/developer/cla/) has been signed
  • CHANGELOG.md updated

@matteomedioli
matteomedioli requested a review from a team as a code owner July 17, 2026 11:35
@matteomedioli
matteomedioli marked this pull request as draft July 17, 2026 11:39
@matteomedioli
matteomedioli force-pushed the matteo/split-http-client-helper branch from 0b4426b to 4baa924 Compare July 17, 2026 15:02
@matteomedioli
matteomedioli force-pushed the matteo/base-gemini-llm branch from 4183734 to 1622fed Compare July 17, 2026 15:03
@matteomedioli
matteomedioli force-pushed the matteo/split-http-client-helper branch from 4baa924 to 2b6a1f6 Compare July 17, 2026 15:07
@matteomedioli
matteomedioli force-pushed the matteo/base-gemini-llm branch from 1622fed to 6510347 Compare July 17, 2026 15:07
@matteomedioli
matteomedioli force-pushed the matteo/split-http-client-helper branch from 2b6a1f6 to 5a6541c Compare July 17, 2026 15:21
@matteomedioli
matteomedioli force-pushed the matteo/base-gemini-llm branch from 6510347 to 44e0906 Compare July 17, 2026 15:21
@matteomedioli
matteomedioli force-pushed the matteo/split-http-client-helper branch from 5a6541c to a819760 Compare July 22, 2026 11:42
Base automatically changed from matteo/split-http-client-helper to main July 22, 2026 13:47
@matteomedioli
matteomedioli changed the base branch from main to matteo/base-anthropic-llm July 22, 2026 16:07
@matteomedioli
matteomedioli force-pushed the matteo/base-gemini-llm branch from 44e0906 to ebfbf3b Compare July 22, 2026 16:10
@matteomedioli
matteomedioli marked this pull request as ready for review July 22, 2026 16:14
@matteomedioli matteomedioli changed the title feat(llm): extract BaseGeminiLLM, export extension point feat(llm): extract BaseGeminiLLM, add base_url support Jul 22, 2026
raise LLMGenerationError(f"Error calling GeminiLLM with tools: {e}") from e

async def aclose(self) -> None:
self.client.close()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird that we need both. Do they also have both a sync and an async connection internally?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems so:

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())

Base automatically changed from matteo/base-anthropic-llm to main July 23, 2026 09:09
…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
matteomedioli force-pushed the matteo/base-gemini-llm branch from f943b3e to 5ca3ff9 Compare July 23, 2026 12:24
@matteomedioli
matteomedioli merged commit 5a78f7e into main Jul 24, 2026
16 checks passed
@matteomedioli
matteomedioli deleted the matteo/base-gemini-llm branch July 24, 2026 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants