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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: the-hercules <thehercules@git.wordpress.org>
Co-authored-by: dkotter <dkotter@git.wordpress.org>
Co-authored-by: jeffpaul <jeffpaul@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.68%. Comparing base (b15cb96) to head (ca265c1).

@@              Coverage Diff              @@
##             develop     #770      +/-   ##
=============================================
+ Coverage      78.44%   79.68%   +1.24%     
- Complexity      2454     2460       +6     
=============================================
  Files            104      104              
  Lines           9925     9955      +30     
=============================================
+ Hits            7786     7933     +147     
+ Misses          2139     2022     -117     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

Hi @dkotter
Thanks for the review! You make a great point about maintainability and relying on developers to remember to manually add apply_filters into every new ability is definitely a recipe for them getting missed over time.

I looked into moving get_prompt_builder() entirely into Abstract_Ability so it's handled 100% automatically as you suggested. The tricky part is that different abilities configure the Prompt_Builder in very specific ways before the filter is meant to run. For example:

• Summarization sets ->using_temperature( 0.9 )
• Content_Classification sets ->as_json_response( ... )
• Generate_Image sets RequestOptions with a 90s timeout and attaches reference images.

If Abstract_Ability::get_prompt_builder() creates the builder and immediately fires the filter hook, individual abilities would have to append their configurations afterwards. That would defeat the purpose of the hooks, because third-party developers wouldn't be able to override things like temperature if the ability hardcodes it after the filter runs.

Proposed Compromise
Instead of a single get_prompt_builder() that builds from scratch, what if we centralize the filter logic into a helper method in Abstract_Ability?

We could introduce a filter_prompt_builder() method on the base class:

protected function filter_prompt_builder( \WP_AI_Client_Prompt_Builder $builder, string $feature_class, array $fallback_models = array(), ...$filter_args ) {
    // 1. Apply model preferences
    $builder = $this->set_provider_model_preference( $builder, $feature_class, $fallback_models );
    
    // 2. Automatically apply the builder filter
    return apply_filters( "wpai_{$this->get_ability_slug()}_prompt_builder", $builder, ...$filter_args );
}

Then, an individual ability simply configures its specific settings and passes the builder through the helper:

$prompt_builder = wp_ai_client_prompt( $prompt )
    ->using_system_instruction( $this->get_system_instruction() )
    ->using_temperature( 0.7 );

// One standardized call that applies the model fallbacks AND fires the filter hook
$prompt_builder = $this->filter_prompt_builder( $prompt_builder, Title_Generation_Experiment::class, array(), $prompt );

We could do something similar for the prompt string itself with a filter_prompt() helper.

This drastically reduces the boilerplate across the abilities and centralizes the hook logic. While new abilities will still have to call $this->filter_prompt_builder(), they already have to call $this->set_provider_model_preference() anyway, so combining them makes it very hard to forget the filters.

Let me know if this approach sounds good to you, and I'll update the PR!

That's a decently thought out approach and I'm not opposed to it. One other solution to consider after looking at things again is to add this filter in the existing ensure_*_supported methods, as I believe every Ability calls that and it's expected any new Ability added in the future will call that. So this wouldn't require a new method to be called and if we run the filter first and then run the supported check, we also ensure any changes someone makes to the prompt builder are supported by a configured model.

So I'd suggest still adding a new filter_prompt_builder method that runs the filter and then use that in the supported methods:

private function filter_prompt_builder( $prompt_builder ) {
      $ability_slug = $this->get_ability_slug();

      $filtered = apply_filters( "wpai_prompt_builder_{$ability_slug}", $prompt_builder, $this );

      // Guard against a filter returning a non-builder (null, WP_Error, etc.).
      if ( ! $filtered instanceof \WP_AI_Client_Prompt_Builder ) {
              return $prompt_builder;
      }

      return $filtered;
}

protected function ensure_text_generation_supported( $prompt_builder, string $message ) {
      $prompt_builder = $this->filter_prompt_builder( $prompt_builder );

      if ( ! $prompt_builder->is_supported_for_text_generation() ) {
              return new WP_Error( 'unsupported_model', $message );
      }

      return $prompt_builder;
}

@dkotter
That’s a good idea to reduce boilerplate, the process of trying to handle the filter automatically inside ensure_*_supported().

However, I'd lean toward keeping a dedicated filter_prompt_builder() helper instead, for two main reasons:

1. Loss of Explicit Context Variables
Right now, some abilities pass highly useful, explicit variables to the filter. For example, Summarization passes the $length argument alongside the prompt:
apply_filters( "wpai_..._prompt_builder", $builder, $prompt, $length );

This allows a third-party developer to easily check if the requested summary is 'short' or 'long' and adjust the builder accordingly. If we bury the filter inside ensure_text_generation_supported( $builder, $message ), the hook loses access to those specific variables.

While we could pass $this, the Ability classes are stateless during a request (they don't store $length as a class property). This means a developer inspecting $this still cannot access the $length for the current execution. The only way to fix this would be to update the signature of ensure_text_generation_supported() to accept ...$filter_args—but if we do that, we have to manually update all 12 ability files to pass those arguments down anyway, which defeats the boilerplate-reduction goal of the suggestion!

2. Separation of Concerns
A method named ensure_*_supported strongly signals that it's strictly a validation check. Firing a powerful, mutating developer hook inside a validation method is a side-effect that might surprise future contributors. Keeping the hook in a dedicated filter_prompt_builder() helper makes the intent of the code much clearer.

Since new abilities will have to call $this->set_provider_model_preference() anyway, wrapping that inside a dedicated $this->filter_prompt_builder() still centralizes the logic and makes it very hard to forget.

Let me know what you think.

@the-hercules This all sounds like valid arguments to me so happy to review and test things once changes are up. Thanks!

@dkotter I’ve committed the changes you requested. Could you please review them when you get a chance?