ADFA-5095 | Publish optional backend capabilities on the LLM inference contract - #1660
ADFA-5095 | Publish optional backend capabilities on the LLM inference contract#1660jatezzz wants to merge 7 commits into
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 Walkthrough
WalkthroughThe LLM inference plugin API adds backend lifecycle and capability contracts, tool-aware chat models, system-prompt support, tool-streaming callbacks, documentation, and validation tests. Plugin context tests are reformatted and their test double gains directory and preference APIs. ChangesLLM backend API
Plugin test maintenance
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: ⚪ Minimal · up to The PR adds optional backend capability and configuration contracts without any supplied evidence of a current merge-blocking issue; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant LlmInferenceService
participant ToolCallingBackend
participant ToolStreamCallback
LlmInferenceService->>ToolCallingBackend: generateStreamingWithTools(...)
ToolCallingBackend->>ToolStreamCallback: onToken(...)
ToolCallingBackend->>ToolStreamCallback: onToolCall(...)
ToolCallingBackend->>ToolStreamCallback: onComplete(...)
ToolCallingBackend->>ToolStreamCallback: onError(...)
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
plugin-api/src/main/java/com/itsaky/androidide/plugins/services/LlmInferenceService.java (3)
196-201: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDocument the storage requirement for
PASSWORDfields.
ConfigFieldType.PASSWORDtells the consumer to mask the input, but the contract does not state how the value must be persisted. The consumer stores the value underkey. State in the Javadoc thatPASSWORDvalues must be stored with EncryptedSharedPreferences or the Android Keystore, and must never be logged or sent to analytics or crash reporting. A published contract is the place to fix this, because each backend author reads it instead of the app code.As per coding guidelines: "Store credentials and signing secrets using EncryptedSharedPreferences or Android Keystore; never commit, log, or send secrets to analytics or crash reporting."
📝 Proposed doc addition
enum ConfigFieldType { + /** + * Text, password, file path, dropdown selection and boolean field kinds. A {`@code` PASSWORD} value is a + * credential: the consumer must persist it with EncryptedSharedPreferences or the Android Keystore, and must + * never log it or send it to analytics or crash reporting. + */ TEXT, PASSWORD, FILE_PICKER, DROPDOWN, BOOLEAN }Also applies to: 238-243
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin-api/src/main/java/com/itsaky/androidide/plugins/services/LlmInferenceService.java` around lines 196 - 201, Update the Javadoc for ConfigFieldType.PASSWORD and its required field contract to state that password values stored under key must use EncryptedSharedPreferences or the Android Keystore, and must never be logged or sent to analytics or crash reporting.Source: Coding guidelines
618-641: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
@paramtags to the callback methods.
ToolStreamCallbackdocuments each method with one sentence but omits@paramtags.StreamCallbackat lines 524-548 documents every parameter. Match that level for the published contract, in particular foronToolCall(ToolCallRequest), where the caller needs to know whether it must respond and on which thread the method is invoked.As per coding guidelines: "Public classes, functions, and non-obvious logic must have KDoc or Javadoc documenting contracts, rationale, threading, nullability, side effects, or units."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin-api/src/main/java/com/itsaky/androidide/plugins/services/LlmInferenceService.java` around lines 618 - 641, Add Javadoc `@param` tags to each parameter of ToolStreamCallback methods, matching the detail and contract style used by StreamCallback. Document the token, error, response, and especially ToolCallRequest parameters, including whether the caller must respond and the invocation thread where applicable.Source: Coding guidelines
310-341: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider reporting the unsupported case through the callback instead of throwing.
Both defaults throw
UnsupportedOperationExceptionfrom a void, callback-based method. Every other failure in this API reaches the caller throughcallback.onError(String). A caller that omits thesupportsHistory()orsupportsTools()check therefore gets an exception on the calling thread instead of an error event, and that exception reaches the global crash handler.The documented rationale for not falling back to plain streaming is sound. Reporting the same refusal through
callback.onError(...)keeps that rationale and keeps the failure inside the established error path.As per coding guidelines: "Catch recoverable I/O, parsing, IPC, git, and plugin failures locally; convert them into explicit error states, never allow unexpected exceptions to reach the global GlitchTip crash handler."
♻️ Proposed change for the history default
default void generateStreamingWithHistory( List<ChatMessage> history, String prompt, LlmConfig config, StreamCallback callback) { - throw new UnsupportedOperationException("Multi-turn history is not supported by backend: " + getId()); + callback.onError("Multi-turn history is not supported by backend: " + getId()); }Apply the same change to
generateStreamingWithTools, and update the@throwstags to describe the error event.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin-api/src/main/java/com/itsaky/androidide/plugins/services/LlmInferenceService.java` around lines 310 - 341, Update the default implementations of generateStreamingWithHistory and generateStreamingWithTools to report unsupported operations through the respective callback’s onError(String) method instead of throwing UnsupportedOperationException. Preserve the existing refusal messages and update both Javadocs’ `@throws` documentation to describe the callback error event rather than an exception.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@plugin-api/src/main/java/com/itsaky/androidide/plugins/services/LlmInferenceService.java`:
- Around line 359-369: Annotate the four nullable default methods in
LlmInferenceService and their parameters as requested: at
plugin-api/src/main/java/com/itsaky/androidide/plugins/services/LlmInferenceService.java:359-369,
add `@Nullable` to getDefaultTemperature() and document that callers must
null-check before assigning to primitive LlmConfig.temperature; at :385-397, add
`@Nullable` to getSystemPrompt(SystemPromptRequest) and `@NonNull` to request; at
:256-266, add `@Nullable` to getSettingsFragmentClassName() and `@NonNull` to
getConfigSpecs(); and at :111-121, add `@Nullable` to getPreferredBackendId().
- Around line 111-121: Annotate the default method getPreferredBackendId() with
the project’s existing `@Nullable` annotation, matching the nullability convention
used by getBackend and preserving its documented null return behavior.
- Around line 588-616: Update ToolCallRequest and ToolDefinition to use final
fields, add Javadoc for each public field, constructor, and constructor
parameter, and defensively copy args and parametersSchema into immutable or
unmodifiable map instances while preserving map order as required. Match the
defensive-copy conventions used by ConfigFieldSpec and SystemPromptRequest, add
any needed LinkedHashMap import, and regenerate the affected API dump.
- Around line 17-20: Update the newly added abstract methods in
LlmInferenceService, including cancelGeneration, to provide default
implementations so existing external implementations remain source- and
runtime-compatible. Preserve the current contract while avoiding mandatory
overrides; only treat this as a breaking change if the project’s compatibility
policy explicitly requires documenting and versioning it.
---
Nitpick comments:
In
`@plugin-api/src/main/java/com/itsaky/androidide/plugins/services/LlmInferenceService.java`:
- Around line 196-201: Update the Javadoc for ConfigFieldType.PASSWORD and its
required field contract to state that password values stored under key must use
EncryptedSharedPreferences or the Android Keystore, and must never be logged or
sent to analytics or crash reporting.
- Around line 618-641: Add Javadoc `@param` tags to each parameter of
ToolStreamCallback methods, matching the detail and contract style used by
StreamCallback. Document the token, error, response, and especially
ToolCallRequest parameters, including whether the caller must respond and the
invocation thread where applicable.
- Around line 310-341: Update the default implementations of
generateStreamingWithHistory and generateStreamingWithTools to report
unsupported operations through the respective callback’s onError(String) method
instead of throwing UnsupportedOperationException. Preserve the existing refusal
messages and update both Javadocs’ `@throws` documentation to describe the
callback error event rather than an exception.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 44a19cd7-2669-438a-bb29-b705bc2d101a
📒 Files selected for processing (2)
plugin-api/api/plugin-api.apiplugin-api/src/main/java/com/itsaky/androidide/plugins/services/LlmInferenceService.java
ce89dd1 to
90d5364
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/PLUGIN_API_CHANGELOG.md`:
- Around line 37-60: Clarify the compatibility wording in the “Optional LLM
backend capabilities” and “Backend-owned settings” entries: state that only
methods added to existing interfaces are default methods, while backends
adopting optional interfaces must implement abstract methods such as
CancellableBackend.cancelStreaming and ConfigurableBackend.getConfigSpecs. Keep
the existing API references and do not add caller gating changes.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0b32cc4c-089a-4271-b2c1-f42fe1e45b6d
📒 Files selected for processing (2)
docs/PLUGIN_API_CHANGELOG.mdplugin-api/src/main/java/com/itsaky/androidide/plugins/services/LlmInferenceService.java
🚧 Files skipped from review as they are similar to previous changes (1)
- plugin-api/src/main/java/com/itsaky/androidide/plugins/services/LlmInferenceService.java
3c98628 to
b9de41a
Compare
Review: eight findings still openReviewed at These eight are not. Contract design1. 2. Tool calling has no return path. 3. 4. Kotlin-consumer correctness5. Nullable public fields are unannotated. The nullability commit annotated constructor parameters and left the fields bare: 6. "Never null, never mutable" overstates what the constructors deliver. 7. Process8. Eleven new public symbols are frozen into the ABI with none of the checklist run. The diff is two files. Findings 1-4 are shape changes worth settling before this ABI is frozen; 5-7 are small and mechanical; 8 is the checklist. |
Review on #1660: capability interfaces replace the flag/method pairs that could disagree, tool results get a return path, describe-only config specs drop ToolDefinition and ToolCallRequest turn immutable, and :plugin-api's tests compile for the first time.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
docs/plugin-api.md (1)
15-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the two streaming callback interfaces to this list.
A backend receives
StreamCallbackandToolStreamCallbackand must call them, so both are part of the contract a backend binds to. The bullet lists the backend interfaces and the value types but omits the callbacks.📝 Proposed wording change
- - Cross-plugin service interfaces, where **one plugin implements what another calls** (via `SharedServices`): `LlmInferenceService` — implemented by ai-core, called by every AI plugin — together with the types nested in it that a *backend* plugin implements (`LlmBackend`, `HistoryCapableBackend`, `ToolCallingBackend`, `CancellableBackend`, `ConfigurableBackend`) and the value types either side constructs (`ChatMessage`, `LlmConfig`, `LlmResponse`, `SystemPromptRequest`, `ToolDefinition`, `ToolCallRequest`). + - Cross-plugin service interfaces, where **one plugin implements what another calls** (via `SharedServices`): `LlmInferenceService` — implemented by ai-core, called by every AI plugin — together with the types nested in it that a *backend* plugin implements (`LlmBackend`, `HistoryCapableBackend`, `ToolCallingBackend`, `CancellableBackend`, `ConfigurableBackend`), the callbacks a backend invokes (`StreamCallback`, `ToolStreamCallback`), and the value types either side constructs (`ChatMessage`, `LlmConfig`, `LlmResponse`, `SystemPromptRequest`, `ToolDefinition`, `ToolCallRequest`).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/plugin-api.md` at line 15, Update the cross-plugin service interfaces list in the documentation to include both StreamCallback and ToolStreamCallback alongside the existing backend interfaces and value types, reflecting that backends receive and invoke these callbacks.plugin-api/src/test/java/com/itsaky/androidide/plugins/services/LlmInferenceServiceTest.java (2)
31-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the
try/fail/catchblocks withassertThrows.Use JUnit 4.13.2’s
org.junit.Assert.assertThrows; capture its returned exception where the test checks the message.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin-api/src/test/java/com/itsaky/androidide/plugins/services/LlmInferenceServiceTest.java` around lines 31 - 59, Update the three tests—chatMessageRejectsANullRole, chatMessageRejectsAToolRoleWithoutCorrelators, and llmConfigRejectsAMissingBackendId—to use JUnit 4.13.2’s org.junit.Assert.assertThrows instead of try/fail/catch blocks. Capture the returned IllegalArgumentException in the tests that verify error messages, preserving the existing exception types and message assertions.
1-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse Truth assertions, but retain JUnit 4.
Because
:plugin-apiuses JUnit 4.13.2 and has no JUnit Platform setup, do not migrate this file to Jupiter. Replaceorg.junit.Assertassertions with Truth and addlibs.tests.google.truthif required.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin-api/src/test/java/com/itsaky/androidide/plugins/services/LlmInferenceServiceTest.java` around lines 1 - 19, Update LlmInferenceServiceTest to retain JUnit 4 while replacing all org.junit.Assert usages with Google Truth assertions, including adapting assertion forms as needed for Truth’s API. Add the libs.tests.google.truth dependency to the plugin-api test configuration if it is not already available.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugin-api/api/plugin-api.api`:
- Around line 1586-1588: Revert the public field declarations for
ToolCallRequest and ToolDefinition in LlmInferenceService so args, callId, name,
description, and parametersSchema remain non-final and mutable. Regenerate the
plugin-api API dump to match these declarations; defer any immutability change
to a separately scoped breaking change with a breaking changelog entry.
In
`@plugin-api/src/main/java/com/itsaky/androidide/plugins/services/LlmInferenceService.java`:
- Around line 585-660: Restore binary-compatible mutability in ToolCallRequest
and ToolDefinition by removing final from all six public fields, removing
defensive map copies and the Objects.requireNonNull checks for callId, name, and
description, and preserving the ownership/mutation warnings in their Javadocs;
update plugin-api/api/plugin-api.api entries at 1586-1599 to public field,
remove only the immutability and null-rejection tests in
plugin-api/src/test/java/com/itsaky/androidide/plugins/services/LlmInferenceServiceTest.java:127-200
while retaining SystemPromptRequest and ChatMessage coverage, and track any
future immutability change separately with a breaking row in
docs/PLUGIN_API_CHANGELOG.md.
- Around line 242-253: Update PluginFragmentFactory to validate each
ConfigurableBackend settings fragment before instantiation: require a Fragment
subclass, load it with the selected plugin classloader, and require a public
no-argument constructor. Reject invalid classes before creation while preserving
the host’s existing instantiation-failure handling.
---
Nitpick comments:
In `@docs/plugin-api.md`:
- Line 15: Update the cross-plugin service interfaces list in the documentation
to include both StreamCallback and ToolStreamCallback alongside the existing
backend interfaces and value types, reflecting that backends receive and invoke
these callbacks.
In
`@plugin-api/src/test/java/com/itsaky/androidide/plugins/services/LlmInferenceServiceTest.java`:
- Around line 31-59: Update the three tests—chatMessageRejectsANullRole,
chatMessageRejectsAToolRoleWithoutCorrelators, and
llmConfigRejectsAMissingBackendId—to use JUnit 4.13.2’s
org.junit.Assert.assertThrows instead of try/fail/catch blocks. Capture the
returned IllegalArgumentException in the tests that verify error messages,
preserving the existing exception types and message assertions.
- Around line 1-19: Update LlmInferenceServiceTest to retain JUnit 4 while
replacing all org.junit.Assert usages with Google Truth assertions, including
adapting assertion forms as needed for Truth’s API. Add the
libs.tests.google.truth dependency to the plugin-api test configuration if it is
not already available.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 31a50219-7927-4fcb-836b-23af599d562e
📒 Files selected for processing (8)
docs/plugin-api.mdplugin-api/api/plugin-api.apiplugin-api/src/main/java/com/itsaky/androidide/plugins/services/LlmInferenceService.javaplugin-api/src/test/java/com/itsaky/androidide/plugins/services/IdeFileServiceTest.javaplugin-api/src/test/java/com/itsaky/androidide/plugins/services/IdeProjectServiceTest.javaplugin-api/src/test/java/com/itsaky/androidide/plugins/services/IdeResourceServiceTest.javaplugin-api/src/test/java/com/itsaky/androidide/plugins/services/LlmInferenceServiceTest.javaplugin-api/src/test/kotlin/com/itsaky/androidide/plugins/PluginContextTest.kt
💤 Files with no reviewable changes (3)
- plugin-api/src/test/java/com/itsaky/androidide/plugins/services/IdeResourceServiceTest.java
- plugin-api/src/test/java/com/itsaky/androidide/plugins/services/IdeFileServiceTest.java
- plugin-api/src/test/java/com/itsaky/androidide/plugins/services/IdeProjectServiceTest.java
|
Please update the PLUGIN_API_CHANGELOG.md file |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugin-api/src/test/java/com/itsaky/androidide/plugins/services/LlmInferenceServiceTest.java (2)
141-159: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTest every null precondition of
ChatMessage.toolResult.The test covers a missing
toolCallIdonly. The production contract also rejects nulltoolNameand nullcontentinplugin-api/src/main/java/com/itsaky/androidide/plugins/services/LlmInferenceService.java, Lines 165-236. Add separate assertions for both inputs.As per coding guidelines: "Use unit tests for non-UI logic, cover error and edge paths, and target at least 50% line and branch coverage for new or changed non-UI code."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin-api/src/test/java/com/itsaky/androidide/plugins/services/LlmInferenceServiceTest.java` around lines 141 - 159, Extend the ChatMessage.toolResult null-precondition tests by adding separate assertions that null toolName and null content each throw NullPointerException, alongside the existing toolCallId test. Keep the valid toolResult assertions in toolResultCarriesBothCorrelators unchanged.Source: Coding guidelines
1-20: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winConfigure
plugin-apifor JUnit Jupiter and Truth.
plugin-apicurrently declares only JUnit 4.13.2 and does not enable the Jupiter platform. Add the existing Jupiter, platform launcher, and Truth dependencies, configure the test task, and migrateLlmInferenceServiceTest.java.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin-api/src/test/java/com/itsaky/androidide/plugins/services/LlmInferenceServiceTest.java` around lines 1 - 20, Configure the plugin-api test setup to use JUnit Jupiter and Truth by adding the existing Jupiter, platform launcher, and Truth dependencies, enabling the Jupiter platform on the test task, and migrating LlmInferenceServiceTest from JUnit 4 assertions and annotations to the corresponding Jupiter and Truth APIs.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/PLUGIN_API_CHANGELOG.md`:
- Around line 55-62: Update the “Tool results correlated by call id” changelog
entry to describe correlation using both toolCallId and toolName, reflecting
that providers may key results by either value. Revise the heading and
explanatory prose while preserving the existing API symbols and Kotlin Role.TOOL
guidance.
---
Outside diff comments:
In
`@plugin-api/src/test/java/com/itsaky/androidide/plugins/services/LlmInferenceServiceTest.java`:
- Around line 141-159: Extend the ChatMessage.toolResult null-precondition tests
by adding separate assertions that null toolName and null content each throw
NullPointerException, alongside the existing toolCallId test. Keep the valid
toolResult assertions in toolResultCarriesBothCorrelators unchanged.
- Around line 1-20: Configure the plugin-api test setup to use JUnit Jupiter and
Truth by adding the existing Jupiter, platform launcher, and Truth dependencies,
enabling the Jupiter platform on the test task, and migrating
LlmInferenceServiceTest from JUnit 4 assertions and annotations to the
corresponding Jupiter and Truth APIs.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cfeaa78e-728b-497f-8a0f-315d30a2bf94
📒 Files selected for processing (4)
docs/PLUGIN_API_CHANGELOG.mdplugin-api/api/plugin-api.apiplugin-api/src/main/java/com/itsaky/androidide/plugins/services/LlmInferenceService.javaplugin-api/src/test/java/com/itsaky/androidide/plugins/services/LlmInferenceServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
- plugin-api/api/plugin-api.api
- plugin-api/src/main/java/com/itsaky/androidide/plugins/services/LlmInferenceService.java
…alling An LlmBackend could only stream a single prompt, so anything model-specific -- prompt wording, temperature, settings UI -- had to be guessed by the consumer or read out of another plugin's preferences. Add, all as defaults so existing backends are untouched: - generateStreamingWithHistory / generateStreamingWithTools, degrading to plain streaming rather than failing - getSystemPrompt(SystemPromptRequest) and getDefaultTemperature: the consumer supplies the tool contract, the backend supplies the wording - getConfigSpecs (ConfigFieldSpec / ConfigFieldType) or getSettingsFragmentClassName for a backend that owns its own settings screen - CancellableBackend for stopping an in-flight stream - LlmInferenceService.getPreferredBackendId, so a costly backend can tell whether it is the one about to be used
…dels getDefaultTemperature() returns a boxed Float where LlmConfig.temperature is primitive, so the obvious assignment unboxes null and throws, now annotated and stated in the Javadoc, along with the four other unannotated members.
Review on #1660: capability interfaces replace the flag/method pairs that could disagree, tool results get a return path, describe-only config specs drop ToolDefinition and ToolCallRequest turn immutable, and :plugin-api's tests compile for the first time.
The last commit tightened ToolCallRequest and ToolDefinition to final fields with defensive copies. Both shipped non-final in 26.28, so that turned an additive PR into an ABI break: an already-built .cgp assigning one of those fields throws IllegalAccessError on putfield, and the null-checks and unmodifiable maps change behaviour for callers that were within contract before. Reverted to the published shape; the hazard the tightening was aimed at is now an ownership rule in the Javadoc instead -- who owns an instance, and that SystemPromptRequest copies only the list spine. Also adds the 26.33 changelog entry the additions were missing, so a plugin author has a min_ide_version to floor at, including the note that Role.TOOL can break an exhaustive Kotlin `when`.
a9119cf to
eadaabb
Compare
Two compatibility breaks that CI can't seeFlagging these because the PR description says "All changes are additive and binary-compatible with existing AI plugins", and both of these falsify it against the plugins we actually ship. 1.
|
Both need a source change in plugin repos, so drop the blanket "every change is additive" claim, define the `breaking` legend entry, and document the previously unlisted nullability change.
|
Good catch on both. The changelog now files |
3 participants
Description
Extended the
LlmInferenceServicecontract in the CodeOnTheGo plugin API so LLM backends can ship as independent plugins. This PR introduces opt-in interfaces for optional capabilities like cancellable streaming, multi-turn history, and tool calling, allowing backends to implement only what they support. It also enables backends to declare their own configuration fields (text, password, dropdown, etc.), removing the need for hardcoded provider settings in the consumer application.Details
ConfigurableBackendandConfigFieldSpecto allow backends to define their own settings UI.CancellableBackendinterface for opt-in streaming cancellation.supportsHistory(),supportsTools()) and default interface methods inLlmBackendto avoid forcing implementation of unsupported features.getPreferredBackendId(),getDefaultTemperature(), andgetSystemPrompt()for better backend-owned context management.document_5179206860728698887.mp4
Ticket
ADFA-5095
Observation
The additions to the
LlmBackendinterface usedefaultmethods that throwUnsupportedOperationExceptionfor history and tool generation. Consumers must checksupportsHistory()andsupportsTools()before invoking these methods, successfully removing the need for brittle, reflection-based discovery.