Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

aspilicious created an issue. See original summary.

aspilicious changed the visibility of the branch 3519302-abstract-token-usage to hidden.

aspilicious changed the visibility of the branch 3519302-abstract-token-usage to active.

aspilicious’s picture

Status: Active » Needs review
mrdalesmith’s picture

Status: Needs review » Active

There is no code here to be reviewed, so setting back to Active.

aspilicious’s picture

Status: Active » Needs review
aspilicious’s picture

But it's my first time creating a fork so maybe I need to do something else.

mrdalesmith’s picture

Status: Needs review » Active

There is a fork but no merge request: see https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... which may assist.

aspilicious’s picture

Status: Active » Needs review

Thanks for the information. This should be better.

mrdalesmith’s picture

Code looks good to me: cannot be properly tested or merged until https://www.drupal.org/project/ai/issues/3517618 has been approved and merged, so this will probably sit for a while.

brentg’s picture

Status: Needs review » Needs work

Tested this and came across the following issues

While testing it with the ai_ckeditor module:
Cannot use object of type OpenAI\Responses\StreamResponse as array in Drupal\ai_provider_openai\Plugin\AiProvider\OpenAiProvider->chat() (line 363 of modules/contrib/ai_provider_openai/src/Plugin/AiProvider/OpenAiProvider.php)

The code now assumes that the $response will be an array, and if I'm correctly, the $respons will only be an array if the response is not streamed (see try catch above)

While testing a combination of modules:
Argument #1 ($tokens) must be of type int, null given, called in /var/www/html/web/modules/contrib/ai_provider_openai/src/Plugin/AiProvider/OpenAiProvider.php
So either the setter should allow null as a value (issue in the other patch), or this patch should be adapted to 0 if it's null

aspilicious’s picture

Status: Needs work » Needs review

I tested this and this doesn't crash anymore with the latest changes. But it also doesn't support streams as it doesn't contain the need info.

:(

svendecabooter made their first commit to this issue’s fork.

svendecabooter’s picture

Adding some logic to the MR, in an attempt to also provide support for token usage, in case of streamed responses.
This needs https://www.drupal.org/project/ai/issues/3524435 to be fixed first.

svendecabooter’s picture

svendecabooter’s picture

Added related issues in AI module, that need to be fixed first, before both token usage for both regular chat and streamed chat can be captured.

svendecabooter’s picture

svendecabooter’s picture

anjaliprasannan made their first commit to this issue’s fork.

marcus_johansson’s picture

Status: Needs review » Needs work

Minor comment and before I merge this - it is compatible from my testing with AI Core 1.1.0, correct?

marcus_johansson’s picture

Version: 1.1.x-dev » 1.2.x-dev
marcus_johansson’s picture

Status: Needs work » Fixed

Fixed this for 1.2.x release, getting merged.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.