Skip to content

fix: Make LlmResponse constructor protected for external subclassing#1137

Open
jinnigu wants to merge 1 commit into
google:mainfrom
jinnigu:fix/subclassing
Open

fix: Make LlmResponse constructor protected for external subclassing#1137
jinnigu wants to merge 1 commit into
google:mainfrom
jinnigu:fix/subclassing

Conversation

@jinnigu

@jinnigu jinnigu commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

Problem:
The LlmResponse constructor uses package-private (default) visibility, which prevents users from subclassing LlmResponse outside of the com.google.adk.models package. This blocks custom LLM integrations (e.g., connecting LM Studio or other local models) that need to extend LlmResponse.

Solution:
Change the LlmResponse constructor from package-private to protected. This is the minimal change needed. It allows subclassing from external packages while still preventing arbitrary instantiation (unlike public). The @AutoValue-generated subclass in the same package is unaffected.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Added ExternalLlmResponseSubclassTest in the com.google.adk.models.external package (outside com.google.adk.models) to verify that an external subclass of LlmResponse can be instantiated.

./mvnw -pl core -Dtest=ExternalLlmResponseSubclassTest test
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS

Checklist

  • I have read the CONTRIBUTING.md document.
  • My pull request contains a single commit.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

@damianmomotgoogle

Copy link
Copy Markdown
Contributor

Hi!

Could you explain more on why you aim to subclass this type ?

I don't think we should make LlmResponse's constructor protected — subclassing it isn't the intended extension point.

LlmResponse is an @AutoValue immutable value type. The package-private constructor is deliberate: the only subclass that's meant to exist is the generated AutoValue_LlmResponse.

For the underlying use case (#1134, integrating LM Studio / a custom model), the right extension point is BaseLlm, not the value types. BaseLlm.generateContent(...) returns Flowable<LlmResponse> — you build responses with LlmResponse.builder()...build() rather than subclassing. That's how all existing non-Gemini integrations do it. If a required field is missing on LlmResponse, the fix is to add that field (or use customMetadata()), not to open up subclassing.

@damianmomotgoogle damianmomotgoogle added waiting on reporter Waiting for reaction by reporter. Failing that, maintainers will eventually closed it as stale. and removed needs review labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting on reporter Waiting for reaction by reporter. Failing that, maintainers will eventually closed it as stale.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LlmResponse constructor makes it hard to create new response type outside of package

3 participants