Closed (fixed)
Project:
OpenAI Provider
Version:
1.2.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
15 Apr 2025 at 15:04 UTC
Updated:
28 Aug 2025 at 08:49 UTC
Jump to comment: Most recent
Comments
Comment #4
aspilicious commentedComment #5
mrdalesmith commentedThere is no code here to be reviewed, so setting back to Active.
Comment #6
aspilicious commentedThere is a fork: https://git.drupalcode.org/issue/ai_provider_openai-3519302/-/compare/1....
Comment #7
aspilicious commentedBut it's my first time creating a fork so maybe I need to do something else.
Comment #8
mrdalesmith commentedThere is a fork but no merge request: see https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... which may assist.
Comment #10
aspilicious commentedThanks for the information. This should be better.
Comment #11
mrdalesmith commentedCode 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.
Comment #12
brentgTested 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
Comment #13
aspilicious commentedI 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.
:(
Comment #15
svendecabooterAdding 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.
Comment #16
svendecabooterComment #17
svendecabooterAdded 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.
Comment #18
svendecabooterComment #19
svendecabooterComment #21
marcus_johansson commentedMinor comment and before I merge this - it is compatible from my testing with AI Core 1.1.0, correct?
Comment #22
marcus_johansson commentedComment #23
marcus_johansson commentedFixed this for 1.2.x release, getting merged.