PostStreamingResponseEvent never gets triggered

Created on 28 May 2025, 7 days ago

Problem/Motivation

This problem came up when working on 📌 Add token usage to streamed chat Active .

The method \Drupal\ai\OperationType\Chat\StreamedChatMessageIterator::triggerEvent() never gets called within the AI module, so the event never gets triggered.

Perhaps this could be added within each AI provider implementation, but it would be best if the base class could take of this.

Feedback by Marcus Johansson on Slack regarding this:

hmm, that is a big bug - this is a little bit of added layers because we didn't set the architecture correct from the start. I will look into if we can run it via the base class when iteration is finished, otherwise we will have to add this task on the providers until 2.0.0

Steps to reproduce

- Enable AI CKEditor integration module + AI Provider OpenAI
- Add AI button to CKEditor
- Enable "Fix spelling" for example
- Select a text in CKEditor that needs spelling fixes.
- The action gets performed, but the debugger never enters the method `\Drupal\ai\OperationType\Chat\StreamedChatMessageIterator::triggerEvent`

Proposed resolution

Feedback by Marcus Johansson:

__destruct magic method would most likely work to trigger it

🐛 Bug report
Status

Active

Version

1.1

Component

AI Core module

Created by

🇧🇪Belgium svendecabooter Gent

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @svendecabooter
  • 🇧🇪Belgium svendecabooter Gent

    MR now triggers the event, when the StreamedChatMessageIterator object gets destructed.

  • Pipeline finished with Failed
    7 days ago
    Total: 236s
    #508207
  • 🇧🇪Belgium svendecabooter Gent

    Test PromptJsonDecoderTest seems to fail now, because \Drupal\ai\Event\PostStreamingResponseEvent::__construct doesn't receive a request_thread_id string in its constructor.
    Not sure if this value is NULL in this test by design, or by accident.
    If by design, then the PostStreamingResponseEvent $request_thread_id parameter should be allowed to be nullable?

  • 🇮🇳India sarvjeetsingh

    Hi @svendecabooter,
    The current destructor approach — it might be causing issues since it triggers for all iterator instances, including test objects.

    Would it make sense to consider moving the triggerEvent() call to the ChatOutput constructor instead? That way:

    - It would only fire for actual streaming responses
    - It wouldn’t interfere with test objects or other unintended cases

    Let me know what you think. Thanks!

  • 🇧🇪Belgium svendecabooter Gent

    @sarvjeetsingh:

    I'm not sure if that would work - I'm still tipping my toes into how all of this works...

    It would be something like this, in the ChatOutput constructor?:

        if ($normalized instanceof StreamedChatMessageIteratorInterface) {
          $normalized->triggerEvent();
        }
    

    In my initial testing, that doesn't seem to work. I then get the same error as in the tests:
    TypeError: Drupal\ai\Event\PostStreamingResponseEvent::__construct(): Argument #1 ($request_thread_id) must be of type string, null given

    Not sure if that's something that also needs to be fixed, or if this means this approach is not workable...

  • 🇮🇳India sarvjeetsingh

    Thanks for testing the ChatOutput approach!

    You're right about the NULL `request_thread_id` issue. The PostStreamingResponseEvent `$request_thread_id` parameter might need to be nullable - I think that makes sense.

    We could make that constructor accept nullable string if that would help resolve this issue. That might be the right approach.

  • Pipeline finished with Failed
    2 days ago
    Total: 225s
    #511940
  • 🇮🇳India annmarysruthy

    I’ve made a few updates to fix the missing triggerEvent() call:

    • Made $request_thread_id nullable in PostStreamingResponseEvent to avoid test failures.
    • Added a $eventTriggered flag in StreamedChatMessageIterator to prevent duplicate events.
    • Added null check in triggerEvent() and improved __destruct() for safety.
    • Introduced finishStreaming() method so providers can trigger the event explicitly (recommended over relying on __destruct()).

    Kindly review.

  • Pipeline finished with Failed
    2 days ago
    Total: 208s
    #511962
Production build 0.71.5 2024