Skip to content

Conversation

@ThomasVitale
Copy link
Contributor

@ThomasVitale ThomasVitale commented Nov 2, 2024

  • Introduced null-safety for all ChatClient APIs and verified via extensive auto-tests.
  • Made the final message list computed by ChatClient consistent and predictable across the different options for providing messages (prompt(), messages(), user(), system()) and verified via extensive auto-tests.
  • Added even more auto-tests to cover as many scenarios and edge cases as possible.

* Introduced null-safety for all ChatClient APIs and verified via extensive auto-tests.
* Made the final message list computed by ChatClient consistent and predictable across the different options for providing messages (prompt(), messages(), user(), prompt()) and verified via extensive auto-tests.
* Added even more auto-tests to cover as many scenarios and edge cases as possible.

Signed-off-by: Thomas Vitale <[email protected]>
return new AdvisedRequest(inputRequest.chatModel, inputRequest.userText, inputRequest.systemText,
inputRequest.chatOptions, inputRequest.media, inputRequest.functionNames,
inputRequest.functionCallbacks, inputRequest.messages, inputRequest.userParams,
// Process userText, media and messages before creating the AdvisedRequest.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This piece of logic used to be executed only in one possible branch of execution (when using .prompt(Prompt prompt)). I moved it here so to ensure consistent processing of the messages before creating the AdvisedRequest, no matter how we get here (for example, when using .messages(List<Message> messages), which was previously behaving inconsistently).

Copy link
Contributor

Choose a reason for hiding this comment

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

yeh, good catch.
Initially the ChatClinet from Prompt used to have a very different path, excluded from the advisors.
I've tried to unify it with there unsealed path (by unsealing the prompt) but have missed the ordering of this section.

@tzolov tzolov self-assigned this Nov 2, 2024
@tzolov
Copy link
Contributor

tzolov commented Nov 3, 2024

Very useful improvements for reliability and consistency. Thanks @ThomasVitale

@tzolov
Copy link
Contributor

tzolov commented Nov 3, 2024

Rebased and merged at 048d54b

@tzolov tzolov closed this Nov 3, 2024
@tzolov tzolov added this to the 1.0.0-M4 milestone Nov 3, 2024
@tzolov tzolov added the design label Nov 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants