-
Notifications
You must be signed in to change notification settings - Fork 26
Add fluent API code examples to architecture documentation #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add fluent API code examples to architecture documentation #12
Conversation
This proposed set of changes to WordPress#2 and is doing a few things: 1. Adding Fluent API code examples (similar to those suggested by @JasonTheAdams [here](WordPress#2 (comment))) alongside the Traditional API examples. 2. Proposing a handful of helper methods to simplify the usage of Candidate Count and avoid implementers from needing to understand what _Candidate Count_ means. 3. Representing @JasonTheAdams' suggestion of `AiClient::withImage(string $mimeType, string $base64Blob)` to mimic the input simplicity that the Traditional API supports. 4. Adding the `AiClient::asJsonResponse()` and `AiClient::asArrayResponse()` as a helper methods for `usingOutputMime('application/json')` 5. Adding helper methods for specifying models that support a certain capability or option.
Thanks, @borkweb! I believe we're actually only going with the Fluent API at this point, with the idea we can add the other API style later if it makes sense to do so. Is that right, @felixarntz? |
Ha! Excellent. That was going to be a question I was going to ask - maintaining both would be laborious |
This is a presumptive change based on [the comment](WordPress#12 (comment)) by @JasonTheAdams in WordPress#12 regarding a potential decision to just go fluent. If that is the intent, then this PR is stripping out the Traditional approach for the implementer API.
I think the decision on Thursday was to go with both, in order to compare developer interest/usage in them and eventually decide on the final approach based on that. The fluent API should be the baseline, and we should start with building that, but the traditional method call API should still be part of the planned architecture. I don't think it's a big lift to wrap the fluent API with it anyway. I understand and even share your preference for the fluent API, but as discussed on Thursday we should properly consider both APIs with developers especially in the early stages of this SDK. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@borkweb Thank you for the PR! Overall the examples look great, and they very much clarify how to use the fluent API and how the two APIs differ.
I think there are a few things here that need to be revised, please see my comments below.
Note: For the class diagrams, I didn't leave all the same comments again on the duplicate instances of AiClient
, PromptBuilder
, and MessageBuilder
, but please make sure to keep them in sync after making changes. :)
Co-authored-by: Felix Arntz <[email protected]>
…ration methods have a singular return type.
Ah, then I misread what you were saying in Slack. I interpreted that as:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few last follow up questions, but no real blockers. The key thing to decide on still is #12 (comment).
For reference, sharing here the directly related discussion that happened on Slack: https://wordpress.slack.com/archives/C08TJ8BPULS/p1752512797654719 |
$providerModelsMetadata = AiClient::defaultRegistry()->findModelsMetadataForSupport( | ||
new AiModelRequirements([AiCapability::TEXT_GENERATION]) | ||
); | ||
|
||
$text = AiClient::prompt('Write a 2-verse poem about PHP.') | ||
->withModel( | ||
AiClient::defaultRegistry()->getProviderModel( | ||
$providerModelsMetadata[0]->getProvider()->getId(), | ||
$providerModelsMetadata[0]->getModels()[0]->getId() | ||
) | ||
) | ||
->generateText(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can simplify this nicely!
$providerModelsMetadata = AiClient::defaultRegistry()->findModelsMetadataForSupport( | |
new AiModelRequirements([AiCapability::TEXT_GENERATION]) | |
); | |
$text = AiClient::prompt('Write a 2-verse poem about PHP.') | |
->withModel( | |
AiClient::defaultRegistry()->getProviderModel( | |
$providerModelsMetadata[0]->getProvider()->getId(), | |
$providerModelsMetadata[0]->getModels()[0]->getId() | |
) | |
) | |
->generateText(); | |
$text = AiClient::prompt('Write a 2-verse poem about PHP.') | |
->withModelRequirements(AiCapability::TEXT_GENERATION) | |
->generateText(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JasonTheAdams I commented before that this is explicitly in a section that shows what happens under the hood / how to do it more verbosely. Since that's the intention here, I don't think we should simplify it again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's related to this thread: #12 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh, I see. That's true. I'd like to change the wording in the example itself. I can do that in a subsequent PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@borkweb Thanks for bearing with me here - this looks excellent! 🎉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's so fluent and beautiful! 🥹
This proposed set of changes to #2 and is doing a few things:
AiClient::withImage(string $mimeType, string $base64Blob)
to mimic the input simplicity that the Traditional API supports.AiClient::asJsonResponse()
andAiClient::asArrayResponse()
as a helper methods forusingOutputMime('application/json')
Here's the ARCHITECTURE.md file in a readable format.