🇬🇧United Kingdom @scott_euser

Hmmm I don't quite understand the purpose of this, I think the issue summary needs more detail perhaps? In Embeddingbase::getEmbedding() we already have the Search API Item ID attached to each embedding. What value does attaching it to the raw embedding give us?

Is the reasoning so that when the LLM is generating the embedding it knows the source to be able to bail on generating the raw embeddings in the first place if the content seems inappropriate/malicious?

Given AI Search is still experimental if we match core change policy, we can change things on patch releases: https://www.drupal.org/about/core/policies/core-change-policies/experime... → not sure how often people would actually extend embedding strategy base, but in any case if they do, however we make the change, they may need to update to fill in search API item, so might as well pass it as an argument to to getRawEmbeddings()? I would guess 99% of sites or maybe 99.9% of sites don't extend these, but open to other opinions...

I suppose could be 'Active'... anyone working on this will need to extend the branch from ✨ Exploring AI Prompt management within AI core Active though

Actually I think we need to decide on the @mention variable to use in ✨ AI Prompt Management @mentions type functionality Postponed as we would not want developers to start implementing this and have to change their find and replace later. Though we could have an upcast/downcast approach it would be better if we avoid that by deciding on variable to use now (@, !, #, etc). Let's have that discussion over there.

Added deciding on this as a required task for this MR.

scott_euser → created an issue.

The reason anything within the Element itself is very time consuming is because within the element, ajax system does not work the same as at the higher form level, so it takes a lot of fudging to get it right (+ automated function JS coverage to match as a result)

Thanks, added to the list of follow-ups:

• The element does however filter on the AI Prompt Type bundle, so even if you use AI all over the place, the element itself is limited to the prompts for the particular context.
• The full list of AI prompts extends the standard entity list builder in core which has a pager at 50 items per page.

scott_euser → created an issue.

Not quite sure how to make this work... I guess more extending of the entity autocomplete in core somehow... haven't looked very deeply

This is going to fail as it needs these:

1. #3525065: Widget allows scope to expand beyond restrictions →
2. #3525063: Widget missing schema →

But then still needs work once those are resolved to add a few more autocomplete attempts and prove the restricted results.

scott_euser → created an issue.

scott_euser → created an issue.

scott_euser → created an issue. See original summary → .

1. Shortest I could come up with for a title that explains to the user what the purpose is.
2. Updated the empty option to match that (+ use typical drupal core any/all labelling of `- label -`

Thanks!

scott_euser → created an issue.

scott_euser → created an issue. See original summary → .

scott_euser → created an issue.

Was committed, so changing to fixed. Though maintainers should grant credit to @fjgarlin

Seems like maintainers have changed and D11 release is in place instead on 11x branch

scott_euser → made their first commit to this issue’s fork.

Seems like maintainers have changed and D11 release is in place instead on 11x branch

This is correct and should be applied as return type to getSubscribedEvents in InitSubscriber

scott_euser → created an issue.

scott_euser → created an issue.

Unless strong objections, v2 is fine and users should be encouraged to update to that rather than stay on v1

I cannot find any mention any more of https://api.unstructured.io, only https://api.unstructuredapp.io, so maybe they deprecated or redirected for example. Updated field descriptions & defaults + added helptext about how to use for the ddev contrib unstructured add-on as well into there. Thanks!

Thanks everyone!

Thanks!

scott_euser → made their first commit to this issue’s fork.

Mostly working, though not sure about that failing test whether related or not...

Spacing is off and can't just be fixed with phpcbf, but should be much quicker

To get this for future travellers:

1. Take raw output from the gitlab pipeline and save as pipeline-output.txt
2. grep '^ Using @' pipeline-output.txt > only-annotation-deprecations.txt
3. sed -E -n "s/^[[:space:]]*Using @([^ ]+) .*Use a (.*) attribute instead\..*$/new \\\DrupalRector\\\Drupal10\\\Rector\\\ValueObject\\\AnnotationToAttributeConfiguration('10.0.0', '10.0.0', '\1', '\2'),/p" only-annotation-deprecations.txt > rector-ready.txt
4. (install drupal rector)
5. (add output from the above to drupal rector)
6. vendor/bin/rector -c rector.php process modules/contrib/search_api

Trying this with drupal rector

This looks good to me. I think given 3x is beta, release notes should just make it clear in case anyone is extending any constraints, that they should add the return types + types to their messages in order to remain compatible.

E.g.

class Class1 {
	public string $message = 'Great message.';
}

class Class2 extends Class1 {
	public $message = 'Great message.';
}
$class = new Class2();

Will result in

Fatal error: Type of Class2::$message must be string (as in class Class1)

And

class Class1 {
	public function myFunction(): string {
		return 'hello1';
	}
}

class Class2 extends Class1 {
	public function myFunction() {
		return 'hello2';
	}
}
$class = new Class2();
print $class->myFunction();

Will result in

Fatal error: Declaration of Class2::myFunction() must be compatible with Class1::myFunction(): string

Okay this works now (and actually saves for both single and plural/multi fields).

Thanks for the review! Needs work actually as ajax loaded inputs aren't saving, I clearly didn't test my own work well enough :)

The problem here is not a silently broken site, it is a broken site that you cannot repair, because even in maintenance mode drush cr always fails because of the locks put on cache_config by search_api_views_data triggered by visitors requesting pages. The more visitors we have, the more risks we have of never being able to complete a drush cr on a production site.

Yes, we have had to work around exactly this in high traffic sites that silently failed, temporarily deleting the view and recreating it, e.g.

drush config:delete views.view.example_search_api_view -y
drush cache:rebuild
drush config:import -y
drush cache:rebuild

Which I guess is triggering the views_data rebuild

As a temporary code to discover and self-repair the issue via a cron job we have run this for our specific in case its helpful for others:

  /** @var \Drupal\Component\Plugin\PluginManagerInterface $views_row_manager */
  $views_row_manager = \Drupal::service('plugin.manager.views.row');
  $node_row_definition = $views_row_manager->getDefinition('entity:node', FALSE);
  if (!$node_row_definition || empty($node_row_definition['entity_type'])) {
      /** @var \Drupal\Core\Plugin\CachedDiscoveryClearerInterface $plugin_cache_clearer */
      $plugin_cache_clearer = \Drupal::service('plugin.cache_clearer');
      $plugin_cache_clearer->clearCachedDefinitions();
      /** @var \Drupal\Core\Cache\CacheTagsInvalidatorInterface $cache_tags_invalidator */
      $cache_tags_invalidator = \Drupal::service('cache_tags.invalidator');
      $cache_tags_invalidator->invalidateTags(['views_data']);
      /** @var \Drupal\Core\Cache\CacheBackendInterface $discovery_cache */
      $discovery_cache = \Drupal::service('cache.discovery');
      $discovery_cache->deleteAll();
  }

I am sure this was overkill, but at least gives a new chance for the issue to get fixed when it silently fails, in this case for a Search API View of Nodes.

In terms of moving this forward, would the 'second try' + log instead of exception MR be at least reducing the risk of silent failure for now? Happy to work on this if anyone can point me in the direction of how we might lock the config as outlined in #46.

I think bulk do all could be potential follow-up for those who trust it to get it right without real review process

Here we go:

No activity, unassigning, will push MR for this

Thanks! I think 1.2.0 safer, gives a chance for more testing with a wider group. Added note on how to test to the issue summary.

I think it's this one in AI module you're after https://www.drupal.org/project/ai/issues/3513409 🐛 Warning: Undefined array key "#entity" in Drupal\ai_translate\Form\AiTranslateForm Active Please give that a shot and reopen if not. Thanks!

Hi! Thanks for the contribution, can you make a merge request instead so tests run + easier for us maintaining to review/comment. Thank you!

Aha I see, this is a problem in Milvus - still rather than moving I opened a new one ( 🐛 'Special Fields' created by Search API for Views are not actually fields and cannot be applied as Milvus conditions Active ) as I expect until releases are made its likely people will stumble across this. Will update the issue title as well to make it more findable from the error.

Thanks for digging and trying to figure it out!

scott_euser → created an issue.

how would it be retrieved in any other method of the class if needed?

My point isn't that its not necessarily not needed, but that if it is needed elsewhere (is it??), adding a side effect to getClient() does not seem like the right way to do it, so more that this approach is temporary code to solve specific problem in 1.0.x. Have not looked deeply, but maybe some way in AiVdbProviderPluginManager::createInstance() to construct the provider e.g. in AiVdbProviderClientBase...

Makes sense thank you!

scott_euser → made their first commit to this issue’s fork.

Looks good thanks! Just some minor phpcs issue flagged

scott_euser → created an issue.

Yes meant to work, probably an oversight when Group functionality was merged (was a very long standing issue with many contributors over years) https://www.drupal.org/project/footnotes/issues/3098138 📌 All footnotes available as block for Layout Builder / Paragraph / ... Fixed though I haven't revised that deeply to check.

So each footnote text has a hash of the text in the ID. The ID is unique but when stripped to get just the hash bit, if the hash matches, then the citation should target the same reference as already stored for output. If the hash doesn't exist the citation should cause a new reference to be created.

When "Collapse footnotes" is selected do we expect to see our content annotated with [1], [1], [2] (since we have 2 instances of footnote 1)? Or with [1], [2], [3] where [1] and [2] both link to a single line in the footnotes block that reads something like [1],[2]: "A"?

[1], [1], [2] in that case and that's what FootnotesFilterPluginTest::testCollapseFootnotes() covers. Group is essentially an extension of that to handle multiple formatted texts on the same page (and the JS version, multiple on the same page with lazy loading/ajax loaded content). The thing not part of offline medium is the backlinks since linking back to source references is not a thing offline.

Ultimately the module has to take some opinionated decisions, and if people aren't happy with those opinions, needs to be flexible enough to allow control - but that of course adds complexity, hence the push always for more test coverage or it will end up being impossible for me to merge things confidently, so that was one of the bigger chunks of work when building out the current version. Some footnotes style guides for print media will say [1], [2], [3] and converting [2] to use `Ibid` when its a repeat of [1] - something I can't imagine being easy with the reference being formatted text, but anyways if someone wants to contribute that it'd be a separate feature needing its own test coverage.

Yes CKE5 clipboard pipeline has a known issue in upstream which occasionally causes test failures (they essentially have 2 pipelines, they have a PR open to streamline into one). Retrying test passed.

I also added some basic test coverage for this (though didn't go as far as extending search api's test processor base...) 📌 Add test coverage for search api ignore citations processor Active

scott_euser → created an issue.

Thanks!

scott_euser → made their first commit to this issue’s fork.

Thanks both for figuring out the problem and working out solution! Looks sensible

We should also add to the tests so it stays fixed. Ie, adding an example of two identical texts getting collapsed & not collapsed to FootnotesGroupBlockTest for example. FootnotesFilterPluginTest::testCollapseFootnotes() already has this outside of the block context, so the issue here is no coverage for within the Group block.

scott_euser → created an issue.

Yeah I don't have access, good to know its Kevin, will ask him for it then, thanks!

scott_euser → created an issue.

Updating title since its no longer really exploring and is now merge ready (IMO)

All updated/resolved, thanks! Also added further test coverage.

scott_euser → changed the visibility of the branch 1.1.x to hidden.

Okay cool, let's leave it active then. Not sure we'll get any more drupal 11 fixes given we've got it working in Drupal 11 and there is a stable release for that and gitlab CI monthly pipeline should also notify on pipeline issues, but I'll set a reminder for summer 2026 D12 release and then we'll at least know for certain for the future.

Added some basic test coverage

Thanks! Yep, I think that code is more Drupal as a whole and loading entities rather than something for this module

Yep still an issue in 4.x unfortunately. MR still solves.

scott_euser → made their first commit to this issue’s fork.

Makes sense and thank you for the detailed explanation. I added a documentation page nested within configuration with options + tried to capture your recommendation with examples.

scott_euser → changed the visibility of the branch 3521300-document-tfa-and-rest to hidden.

Thanks, looks good!

The reason the behaviour exists for controlling/overriding it centrally is that sites can have many many site setting types, using manage form display for each one is time consuming via the UI (yes I realise you could bulk do via config/sync/ but I think its fair to say the average site builder won't know how to do that)

Can you provide more info? Maybe the UI needs to be made more clear that you do the configuration in Pinecone and just select the one you created within Drupal

Hmmm not sure that's the right approach because we should be granting credit for the work done. I instead added a scheduled monthly pipeline to Gitlab CI in case there is a deprecation added to D10 that is removed in D11 (though I think that's not possible now and only deprecations would happen in D11 for D12 which would get created as a new issue by the bot?). The scheduled pipeline will also flag deprecations added to D11 that will affect D12.

I'll switch back to fixed for now, but feel free to disagree of course.

Looks good!

The non-JS option will always struggle in some scenarios because the only way a layout builder/experience builder/paragraphs based content can build up the combined footnotes as is is when they get rendered (or at least currently). You may get more mileage with the JS based grouping instead as the numbering, etc is all calculated after the page fully renders including getting recalculated if new elements arrive on the page (e.g. via bigpipe or via ajax).

But yeah if you did ever want something to get in:

1. Needs to be via merge request so coding standards and test coverage gets run
2. Needs to have test coverage run for the change
3. If the change could potentially negatively affect existing sites, needs to have an opt-in feature

Added a small comment, unlikely scenario, but better to be safe

Added some comments to the MR. I still think (per slack) that this should never go to 1.1.x though as getClient() should just get client and not cause side effects. And since 1.1.x has a mechanism to get the server, this is really just temporary throw away code for 1.0.x