Coming from the duplicate issue
📌
Add return types for all methods with a rector rule
Active
here is a basic rector.php config for adding return type hints.
<?php
return Rector\Config\RectorConfig::configure()
// path from composer root.
->withPaths([__DIR__ . '/modules/custom/search_api'])
->withTypeCoverageLevel(10);
We can add the #[ReturnTypeWillChange] attribute to suppress warnings in implementing classes.
I'd like to opt-in https://www.drupal.org/project/ai_vdb_provider_opensearch → as it's a small project that shouldn't disrupt anyone too much.
My 2 cents... if we are injecting services at discovery time, what advantage does this have over regular container services?
kim.pepper → changed the visibility of the branch 3536726-use-callableresolver-for to hidden.
kim.pepper → made their first commit to this issue’s fork.
Can you confirm the version (or commit hash) you are using?
OK. I think this is ready for reviews again.
I went down the path of trying to add CallableResolver to entity form constructor and every sub-class constructor. I think that would be very disruptive. All the unit tests would need to be updated etc. I ended up with checking if the container was available.
Not ideal that Entity form calls a service as it should be a value object like other forms.
Updated the title to be more specific.
I spent a couple of hours looking at how we could use CallableResolver and avoid calling it in FormState.
I deprecated \Drupal\Core\Form\FormStateInterface::prepareCallback() and looked at where it was being called from.
Obvously a lot of test fails 😬 but could be a good first step.
Based on the feedback, I've updated our Time service to implement Psr\Clock\ClockInterface and added a service alias for @datetime.time
Committed to 2.x and 3.x. Thanks!
Deleted the update test as we are no longer updating.
Tests are still failing so I left at NW
If we are mocking the current time I would much prefer us to use ✨ Add PSR-20 Clock Interface as composer dependency Active and the follow-up ✨ Implement PSR-20 in datetime.time service Active
Committed to 3.x and 2.x. Thanks!
Seems like a sensible fix. The phpstan errors are because the baseline format has changed slightly so I regenerated it.
Rebased on 11.x and removed the post-update hook as per #11
Updated deprecations message in two constructors (DirectoryWithMetadataDiscovery and DirectoryWithMetadataPluginDiscovery to be for removal in 12.x.
I think I'm ok to put back to RTBC in this case.
This is ready for reviews now.
Testing this locally, I am getting
illegal_argument_exception: Can't update non dynamic settings [[index.analysis.filter.synonyms.type, index.analysis.analyzer.default.filter, index.analysis.filter.synonyms.lenient, index.analysis.analyzer.default.type, index.analysis.analyzer.default.tokenizer, index.analysis.filter.synonyms.synonyms]] for indices [content]
when saving mappings.
Trying a different approach with a aoss mode setting.
Looks like we will need to sort out ✨ Compatibility with AWS Open Search Serverless (AOSS) Active before we can properly test this. Postponing on that.
Re #10 this was done in 📌 Convert models to plugins, remove hardcoded model string checks Needs review and merged to 1.x.
I have rebased this MR on 1.x
📌 Convert models to plugins, remove hardcoded model string checks Needs review is in so this is unblocked.
kim.pepper → created an issue.
There's a long list of models. I guess people can create their own plugins if they want a model we don't include here? https://docs.aws.amazon.com/bedrock/latest/userguide/models-supported.html
Great improvement. As discussed in slack we can deprecate titan v1 and add titan v2 in the follow-up 📌 Deprecate titan text embedding v1 and add v2 Closed: duplicate
kim.pepper → created an issue.
kim.pepper → created an issue.
kim.pepper → created an issue.
Getting this error locally and it's failing the test:
Exception: Warning: Undefined array key "connector"
Drupal\ai_vdb_provider_opensearch\Form\OpenSearchConfigForm->validateConnectorConfigForm()() (Line: 125)
That looks like the trait in search_api_opensearch
kim.pepper → created an issue.
Closing this as duplicate.
As well as committing #3545869: Add a ConnectorFormTrait for re-use → I have commited the provider code here to the 1.x branch of https://www.drupal.org/project/ai_vdb_provider_opensearch → in 📌 Use the official opensearch-php library Active
Please continue with remaining work (such as adding tests!) in https://www.drupal.org/project/ai_vdb_provider_opensearch →
All contribution credits have been added to both issues.
Commited to 1.x. Copied commit credits from 📌 Add support for AI module VDB Provider Active
Committed to 3.x and cherry-picked to 2.x.
Added credits from related issue 📌 Add support for AI module VDB Provider Active
kim.pepper → created an issue.
No this is passing, I'm going to split of an issue to just add the trait to the core module for re-use.
Responded to suggestions. I don't think we should be adding new interfaces in this issue just for the sake of autowiring.
Committed to 2.x
There is currently a bug and I can't save contribution records, but I will come back and add them when it's fixed.
kim.pepper → made their first commit to this issue’s fork.
kim.pepper → created an issue.
I added DeprecatedServicePropertyTrait for the removed $fileSystem property.
Some example code:
Create a plain old value object with some constraint attributes:
use Symfony\Component\Validator\Constraints as Assert;
class Embedding {
public function __construct(
#[Assert\Regex(
pattern: '/^[A-Za-z0-9_-]+:[A-Za-z0-9_-]+$/',
message: 'Id should be in the format "prefix:suffix"',
)]
readonly public string $id,
#[Assert\NotBlank(message: 'Values should not be empty')]
public array $values,
#[Assert\NotBlank(message: 'Metadata should not be empty')]
protected array $metadata,
) {}
Create a Symfony validator that looks for the attributes:
$validator = \Symfony\Component\Validator\Validation::createValidatorBuilder()
->enableAttributeMapping()
->getValidator();
Validate the object:
$embedding = new Embedding(
'model1:embedding1',
[0.1, 0.2, 0.3],
['source' => 'test']
);
/** @var \Symfony\Component\Validator\ConstraintViolationListInterface $violations */
$violations = $validator->validate($embedding);
nit: looks like some of the docblock indentations are incorrect. I assume this would be fixed automatically with phpcbf?
@kingdutch wrote a great blog post on how using Symfony Runtime could help solve some of these issues https://www.alexandervarwijk.com/blog/2025-09-08-proposal-restructuring-...
In this issue, I showed how you can create a value object and validate it using Symfony Validator. It's a pretty clean solution IMO. 📌 Create an 'embedding' object that can be validated Active
Rebase on 11.x
I like @danielveza 's idea of
I wonder if we can turn these into plugins or do something a little nicer than elseifs checking strings.
Thanks @marcus_johansson. Can't speak for @danielveza but I will put my hand up for maintainership.
We should decide on whether to fix the getConnector() fails or just duplicate some of the code and remove the trait.
I think this is critical as the Titan V1 embedding model doesn't work at all.
Ok great. Thanks for the feedback.
Random fail
I've been granted maintainership of ai_vdb_provider_opensearch so we should just decide when the best time would be to push this MR over there.
Rebased on 11.x
Thanks for your investigation in #12.
Given that, I think we should close this won't fix. Please re-open if there is an alternative approach.
Yep I think your changes are good.
I'm thinking we should get things working here, then push it back to the ai_vdb_provider_opensearch module. Having it as a sub-module while the API is experimental is going to be problematic. We want to keep releases for the search_api_opensearch module fairly stable, while this might need multiple frequent releases until it's stable.
I've reached out to @fago and @Maximillian Mikus in Slack to see if we can be added as maintainers (if you agree) of ai_vdb_provider_opensearch →
kim.pepper → made their first commit to this issue’s fork.
Should we postpone this on 🐛 SearchApiAiSearchBackend should ask configured VDB provider to supply dependencies Active or can we work around it for now?
Added the special chars to config with a default fallback.
I think this is ready to go.
smustgrave → credited kim.pepper → .
I used xdebug to work through this code locally and the test skips \Normalizer::normalize() because class_exists('Normalizer') returns false.
However, it successfully converts ö to oe in \Drupal\Component\Transliteration\PhpTransliteration::replace() and so the test passes.
I can't really take this any further until we get some input from the transliteration maintainer (@amateescu). I'm changing the component to transliteration system for that.
Test-only pipeline did not fail as expected.
I made a naive attempt at a test. I'm not proficient any any other language besides English, so would like someone to weigh in and ensure we have the right test coverage here.
Ran the test-only pipeline to see if it fails as expected.
Thanks for the explanation and the MR. This needs a test so we can show it failing before and fixed after.
Looks like a straight forward fix.
Marking this as a feature request.