🇮🇳India @vakulrai

Just to add to the above , can we also think of a helper method to Add tracking for retry token usage and retry reasons in AI responses.

My thought is :
we are tracking the mentioned properties but it does not explicitly track retries — which can silently increase token usage and costs when outputs are malformed or invalid (e.g., bad JSON, failed function/tool calls, hallucinated responses, timeouts, etc.).

These retries consume additional tokens and can skew both performance and cost reporting if left untracked.

While total input and output tokens might include retries, but they dont tell:

• How many times a retry occurred
• Why each retry happened
• Which prompt caused it

Can we do it as a feature in AI and tke it ofrard in a seperate ticket if this really can be a good add on.
Open for suggestions

Thanks !

Added a MR to fix the issue , please review
Thanks !

vakulrai → created an issue.

Changes look good to me @michaellander .

Hi @marcus_johansson , Please review the changes and guide me if any other changes needed.

Thanks !

I will check this , assigning to myself.

I have added PHPCS fixes to the files as well. Marking the status to review now.

vakulrai → created an issue.

Mr is updated with the change.

Sure @marcus_johansson, i will update the MR.

vakulrai → created an issue.

vakulrai → created an issue.

vakulrai → created an issue.

I was able to test the feature and i can see its able to pick up the image field by extracting the media reference field.

Test case:
- Created and used media reference field in the Provider setting.
- The Ai output was able to understand the image and able to generate a description.

vakulrai → created an issue.

I tried to debugged why : core/modules/taxonomy/tests/src/Functional/Views/TermTranslationViewsTest.php its faling and that is because the method : taxonomy_term_is_page($term) is checking for the route \Drupal::routeMatch()->getRawParameter('taxonomy_term') and the test is runing on a view that has a contextual filter argument as : arg_0 .

So , the above check $variables['page'] = $variables['view_mode'] == 'full' && taxonomy_term_is_page($term); will always be false , so some additionla logic i needed to that covers the scenario of view.

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

vakulrai → created an issue.

Thank you @marcus_johansson for a review andpointing me to the code.

I have updated the MR accordingly and found one more issue about default value not being passed to "Other Joiner" select type, so i have fixed that in the same mr.

Hope its fine :)

Created a MR to fix the issue , i can see the default value is being considered on the settings form.
Please review !

vakulrai → created an issue.

Hi

Can you please add more information about the issue , like if this is happening on Drupal CMS or vanilla drupal instance ?

I am on Drupal version : 10.4.0 and using the 1.0.3 version of AI but the issue is not reproducible

Thanks !

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

Kristen Pol → credited vakulrai → .

vakulrai → created an issue.

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

All test Passed , moving to review !

All the tests are passing now , moving to ready for review !

I have rerolled to 11.x and have brought down the test failure to 6 :), please review the reroll and suggest path forward to fix the remaining test failures.
Thanks !

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

I have added a fix for the failed test https://git.drupalcode.org/issue/drupal-2910353/-/jobs/676914 , and along with that i saw 2 tests are failing unexpectedly , which looks unrelated to the changes added here , I am adding the failures from the test here for visibility so can someone do a quick review and suggest a path forward.

1. https://git.drupalcode.org/issue/drupal-2910353/-/jobs/676912

ErrorException: Method "IteratorAggregate::getIterator()" might add
"\Traversable" as a native return type declaration in the future. Do the
same in implementation "Drupal\Core\Entity\ContentEntityBase" now to avoid
errors or add an explicit @return annotation to suppress this message. in
/builds/issue/drupal-2910353/vendor/symfony/error-handler/DebugClassLoader.php:337

2. https://git.drupalcode.org/issue/drupal-2910353/-/jobs/676917

 RuntimeException: Adding non-existent permissions to a role is not allowed.
    The incorrect permissions are "use text format oqcx2iea".

Thanks!

Updated the comments in the MR , please review !

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

11.x branch has merge conflicts can someone rebase it.

vakulrai → changed the visibility of the branch 3091285-composer-scaffolding-fails-preprocess to hidden.

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

Re-rolled patch as its reproducible for 11.x branch and for the tests as the errors are more due to the JS mutation observer is missing a check and the error in the console cant be tracked in the Tests so can anyone suggest a way to have a test coverage here Else we can move this ticket forward.

Thanks!

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

Converted the patch as a MR , will try to update the https://www.drupal.org/project/drupal/issues/2910353#comment-15031176 🐛 Prevent saving config entities when configuration overrides are applied Needs work in the next commit.

Thanks !

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

Hi @wim-leers i tried to add my comments on the pending feedback , please review.

Points covered :

• https://git.drupalcode.org/project/drupal/-/merge_requests/5132#note_255633
• https://git.drupalcode.org/project/drupal/-/merge_requests/5132#note_255644

Thanks !

I have added the constraint validators to the core/modules/image/config/schema/image.schema.yml , please review.

Thanks !

vakulrai → changed the visibility of the branch 3395616-img-settings-config-validation-constraint to hidden.

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

I tried to reproduce it on 10.x branch by doing the below changes but the issue is not happening anymore:

• I intentionally added chmod 444 to sites/default/default.settings.php file while on a 9.5.x branch
• Moved to Drupal 10.x branch and run composer update but what i can see is that the 444 got converted to 644

I think this ReplaceOp::process() method is managing the files in a correct manner :

/**
   * {@inheritdoc}
   */
  public function process(ScaffoldFilePath $destination, IOInterface $io, ScaffoldOptions $options) {
    $fs = new Filesystem();
    $destination_path = $destination->fullPath();
    // Do nothing if overwrite is 'false' and a file already exists at the
    // destination.
    if ($this->overwrite === FALSE && file_exists($destination_path)) {
      $interpolator = $destination->getInterpolator();
      $io->write($interpolator->interpolate("  - Skip <info>[dest-rel-path]</info> because it already exists and overwrite is <comment>false</comment>."));
      return new ScaffoldResult($destination, FALSE);
    }

    // Get rid of the destination if it exists, and make sure that
    // the directory where it's going to be placed exists.
    $fs->remove($destination_path);
    $fs->ensureDirectoryExists(dirname($destination_path));
    if ($options->symlink()) {
      return $this->symlinkScaffold($destination, $io);
    }
    return $this->copyScaffold($destination, $io);
  }

I think these 2 files are usually should be copied and then updated before any integration and any modification to these files can cause permission hardening.

sites/default/default.services.yml
sites/default/default.settings.php

I came across this problem while moving from 9.5.x to 10.x and i feel + 1 to modify the scaffolding process in a manner that if the permission of incoming version differs from what is locally present try to do the below:

• Store the existing permission and do a chmod 775 on sites/default
• replace or overwrite based on the scaffolding process set in composer.json
• Then restore the existing permission from bullet 1

Else fallback to the default behaviour.

vakulrai → created an issue.

Adding a patch to fix the if condition check.

vakulrai → created an issue.

attaching the patch

vakulrai → created an issue.

Updating the patch for D10 compatability.

Hi @dave Thank you for the suggestions.

Actually the problem that we are facing is for existing content where we have those attributes set already like :
<drupal-entity data-entity-url-title data-no-margin data-entity-url-target>

now the problem is when we move to CKE5 , the users are not able to see the changes controlled by the attributes above, as they were not moved under
data-entity-embed-display-settings automatically .

The idea is there should be a way to migrate the existing attributes to data-entity-embed-display-settings to support the existing functionality.

Thank you !

Attaching the patch for visibility:

Adding a patch to fix the issue , this is definitely can be made configurable but to start off i am adding it so that others facing the same can use it.

Thanks

vakulrai → created an issue. See original summary → .

vakulrai → created an issue.

vakulrai → created an issue.

vakulrai → created an issue.

Uploading the patch diff from : https://git.drupalcode.org/project/token/-/merge_requests/36/diffs for visibility.

I am adding a patch to support the feature , i know this is not the very prominent or reliable solution but just for other projects having the some problem:

vakulrai → created an issue.

vakulrai → created an issue.

Reuploading : used the urong issue id:

Re-rolling the last patch to add the code within the password_policy_update_8304 to avoid any conflicts with the upcoming releases.