Skip to content

Conversation

@fan711
Copy link
Contributor

@fan711 fan711 commented Oct 29, 2025

This allows to provide middleware to Guzzle. For instance a debug logger:

$stack = HandlerStack::create();
$stack->push(\GuzzleHttp\Middleware::log(
    Log::getLogger(),
    new \GuzzleHttp\MessageFormatter('{method} {uri} {code} {req_body:?} {res_body:?}')
));

return new OpenAI(
    ...
    httpOptions: new HttpClientOptions(
        handler: $stack
    )
);

@ilvalerione ilvalerione merged commit e011fe9 into neuron-core:2.x Oct 31, 2025
$expectingRole = ($expectingRole === [MessageRole::USER->value])
? [MessageRole::ASSISTANT->value, MessageRole::MODEL->value]
: [MessageRole::USER->value];
: [MessageRole::ASSISTANT->value, MessageRole::USER->value];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some weird behaviour since this change. I can't see any ToolCallMessage anymore.

Before:
image

After:
image

Is it something expected ?

Copy link
Contributor

@ilvalerione ilvalerione Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you introduced a bug. I didn't notice it because the PR was about the Guzzle provider, but you changed the logic in AbstractChatHistory that is inconsistent I think.

Copy link
Contributor

@ilvalerione ilvalerione Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whay did you add the assistant role at line 205 of the AbstractChatHisotry?

Copy link
Contributor Author

@fan711 fan711 Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not intend to add the second commit to the PR but I pushed it to the same branch so it still slipped in there.

In fact this line solved an issue for me when I have multiple assistant messages in a row. It should be acceptable because I hand the conversation between nodes and these assistant messages are internal 'thinking'. I know it was in earlier days required to have messages strictly alternating between user and assistant but I think it is not a requirement anymore - at least the OpenAI api accepts multiple messages per role.

Thus I propose to remove this restriction of alternating message roles altogether. But if it is not acceptable I would just do it in my fork and I'm sorry to have caused a bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants