-
Notifications
You must be signed in to change notification settings - Fork 28
Add AI_Service layer for centralized AI operations #101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Add AI_Service layer for centralized AI operations #101
Conversation
Implements a service layer that provides a consistent interface for experimental features to communicate with AI providers through the WP AI Client SDK. New files: - includes/Services/AI_Service.php - Singleton service class - includes/Services/Contracts/AI_Service_Interface.php - Interface contract - tests/Integration/Includes/Services/AI_ServiceTest.php - Integration tests Features: - Centralized AI provider configuration and request handling - Model preference caching with clear_model_cache() method - Graceful handling when no provider is configured - Helper function get_ai_service() for easy access Hooks and filters: - ai_service_available: Override provider availability detection - ai_service_initialized: Action when service initializes - ai_service_prompt_builder: Filter prompt builder before generation Closes WordPress#26
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @iTsphillgood. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #101 +/- ##
=============================================
+ Coverage 27.33% 37.16% +9.83%
- Complexity 152 163 +11
=============================================
Files 14 15 +1
Lines 900 931 +31
=============================================
+ Hits 246 346 +100
+ Misses 654 585 -69
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@caseymanos thanks for the PR! I see you note this closes #26, but looking to double confirm that the various goals in that issue description are all covered by your work here; correct? |
felixarntz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While overall I agree that a more opinionated AI service makes sense in the context of this plugin, I think it does too many things. Specifically, it obscures some of the flexibility that the underlying WordPress AI Client APIs provide, for no good reason as far as I can tell.
I think we should simplify the implementation to focus on creating a prompt builder with plugin specific defaults applied, and leave the rest to the regular prompt builder APIs.
| * | ||
| * @since 0.1.0 | ||
| */ | ||
| interface AI_Service_Interface { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why an interface? Seems a bit premature at this point I'd say.
- There's only one
AI_Serviceimplementation. - The interface mandates having methods like
generate_text()which I'm not sure is what makes sense for an interface / contract. There could also be an AI service for generating images for example.
For those reasons, I think it's best to remove this interface entirely. There's no real need for it, at least not at this point.
includes/Services/AI_Service.php
Outdated
| * | ||
| * @var float | ||
| */ | ||
| public const DEFAULT_TEMPERATURE = 0.7; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this the default temperature? More importantly: Why does it even make sense to set a default temperature?
includes/Services/AI_Service.php
Outdated
| * } | ||
| * @return string|\WP_Error The generated text or WP_Error on failure. | ||
| */ | ||
| public function generate_text( string $prompt, array $options = array() ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see the benefit of this method (and generate_texts similarly). Why are we not leveraging the APIs that the WordPress AI Client SDK gives us?
I would suggest for the center piece of this class to be the create_prompt method that sets up a prompt builder with all of the defaults applied. And allow passing an optional array of config options to that method.
What the consuming code then does with that prompt builder is up to them; they can use all the underlying APIs as needed.
Something like:
$prompt_builder = $ai_service->create_prompt( $prompt, $options );
$text = $prompt_builder->generate_text();Or, make it a single statement:
$text = $ai_service->create_prompt( $prompt, $options )->generate_text();
Have you tried Google Jules agent or Gemini 3 on cli? |
Address @felixarntz review comments: - Remove AI_Service_Interface (premature abstraction) - Remove DEFAULT_TEMPERATURE constant (no forced defaults) - Remove generate_text/generate_texts methods (redundant wrappers) - Make create_prompt() the centerpiece with optional $options array - Use existing get_preferred_models() helper instead of duplicating - Simplify from 311 to 181 lines The new API follows Felix's suggested pattern: $text = $ai_service->create_prompt($prompt, $options)->generate_text();
b3bfd72 to
8e0a728
Compare
|
@felixarntz @jeffpaul
Additional improvements:
Updated API// Simple usage - model preferences auto-applied
$text = get_ai_service()->create_prompt('Summarize this...')->generate_text();
// With options array
$text = get_ai_service()->create_prompt('Translate to French', [
'system_instruction' => 'You are a translator.',
'temperature' => 0.3,
])->generate_text();
// Full SDK access via chaining
$titles = get_ai_service()->create_prompt('Generate titles')
->using_candidate_count(5)
->generate_texts();
Hooks Available
| Hook | Type | Description |
|------------------------|--------|------------------------------------------|
| ai_service_available | Filter | Override provider availability detection |
| ai_service_initialized | Action | Fires when AI service initializes |
| ai_preferred_models | Filter | Customize default model preferences |
Note: Removed ai_service_prompt_builder filter as it's no longer needed - create_prompt() returns the builder directly so consumers can modify it with the SDK |
felixarntz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@caseymanos Almost LGTM, just one remaining concern.
includes/Services/AI_Service.php
Outdated
| } | ||
|
|
||
| // Check if any provider credentials are configured. | ||
| $credentials = get_option( 'wp_ai_client_provider_credentials', array() ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using API_Credentials_Manager::OPTION_PROVIDER_CREDENTIALS instead of hard-coding this string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's currently private but eliminating the encompassing is_available() method removes the need to make it public at this time
includes/Services/AI_Service.php
Outdated
| * | ||
| * @return bool True if a provider is available, false otherwise. | ||
| */ | ||
| public function is_available(): bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this method? I understand the point is to return whether any AI is generally available - which maybe makes sense, but I am wary because it's not going to be truly accurate.
The WordPress AI Client already offers a much more precise way of determining that, via the is_supported* methods of Prompt_Builder.
The code example included with the get_ai_service() documentation confirms that including this method could be misleading: It's NOT how one should check whether AI is available.
|
Related: @dkotter @caseymanos I'm not sure about the |
Address Felix's review feedback: - Remove is_available() method from AI_Service (SDK provides is_supported_for_*() methods on Prompt_Builder for capability checking) - Rename get_preferred_models() to get_preferred_models_for_text_generation() to clarify the function returns text-generation-specific models - Rename filter from ai_preferred_models to ai_preferred_models_for_text_generation - Update docstring example to use SDK's is_supported_for_text_generation()
|
@felixarntz Also renamed get_preferred_models() to get_preferred_models_for_text_generation() (filter renamed too). |
felixarntz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@caseymanos Thanks, LGTM!
Note there are some merge conflicts though that need to be resolved.
includes/Services/AI_Service.php
Outdated
| * Manages AI provider configuration and provides a consistent interface | ||
| * for experimental features to communicate with AI providers. | ||
| * | ||
| * @since 0.1.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since 0.1.0 has now been released, these all need updated. Ideal is to use x.x.x for these and they will then be updated as part of our release process
includes/Services/AI_Service.php
Outdated
| * | ||
| * @param \WordPress\AI\Services\AI_Service $service The AI service instance. | ||
| */ | ||
| do_action( 'ai_service_initialized', $this ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've landed on using ai_experiments as our prefix, so need to update that here
includes/Services/AI_Service.php
Outdated
| public function create_prompt( ?string $prompt = null, array $options = array() ): Prompt_Builder_With_WP_Error { | ||
| $builder = AI_Client::prompt_with_wp_error( $prompt ); | ||
|
|
||
| // Apply default model preferences. | ||
| $models = get_preferred_models_for_text_generation(); | ||
| if ( ! empty( $models ) ) { | ||
| $builder = $builder->using_model_preference( ...$models ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future, we will definitely have experiments that do more than just text generation. Since this method forces the preferred models to be text generation only, we won't be able to use this when we add things like image generation. Maybe that's fine but could see either renaming this method to be something like create_text_prompt or have an option you can pass in that marks things as text generation, audio generation or image generation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great catch, +1. How about renaming this to create_textgen_prompt? I think that's less ambiguous, because create_text_prompt could also be interpreted as the input being text. Alternatively create_text_generation_prompt which would be most clear, but maybe a bit long.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I initially had something similar in place when building out the Title Generation Experiment before getting feedback that it made things over-abstracted (see #67 (review)). So may be worth revisiting that discussion to ensure we're all on the same page cc/ @JasonTheAdams
That said, overall I'm fine with this approach but seems we just add the AI_Service class in this PR but we're not actually using it anywhere. Do we want to update how the Title Generation Experiment is working to take advantage of this new approach?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wary of overabstraction here too, which is why the PR has been notably reduced in scope from what it was originally.
I think the way it currently is is fine. Although it mostly feels like syntactic sugar and a slightly alternative way to use the fluent API from the WP AI Client prompt builder.
Address dkotter and felixarntz review feedback: - Rename create_prompt() to create_textgen_prompt() to clarify this method is specifically for text generation - Update @SInCE tags from 0.1.0 to x.x.x for release process - Rename hook from ai_service_initialized to ai_experiments_service_initialized to use correct prefix
|
What?
Closes #26
Implements a centralized AI service layer (
AI_Service) that provides a consistent interface for experimental features to communicate with AI providers through the WP AI Client SDK.Why?
The plugin needs a unified service layer so that experimental features can communicate with AI providers without duplicating logic or directly coupling to the
AI_ClientSDK. This provides:How?
New files:
includes/Services/AI_Service.php- Singleton service class withgenerate_text(),generate_texts(),create_prompt(),is_available()methodsincludes/Services/Contracts/AI_Service_Interface.php- Interface contract ensuring consistent implementationtests/Integration/Includes/Services/AI_ServiceTest.php- Integration tests (26 test cases)Modified files:
includes/bootstrap.php- Initializes AI_Service afterAI_Client::init()includes/helpers.php- Addsget_ai_service()helper functionHooks and filters:
ai_service_availableai_service_initializedai_service_prompt_builderTesting Instructions
npm run test:phpis_available()should returnfalseTesting Instructions for Keyboard
Not applicable - this PR adds backend service infrastructure only with no UI changes.
Screenshots or screencast
Not applicable - this PR adds backend service infrastructure only with no UI changes.