-
Notifications
You must be signed in to change notification settings - Fork 99
Add calls.mdx, iterate on Response api #1094
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I think it is a lot more intuitive if llm.call always returns `llm.Response`, which has everything you expect defined there, and what users mostly think about. Then in the specific case where they are using tools, they can specify whether they expect `llm.SimpleResponse` or `llm.ContextResponse`. This is more intuitive than making `llm.BaseResponse` the class they are always expecting (and more accurate than treating the SimpleResponse as the default, because that isn't consistently accurate).
I think this is just a more intuitive/useful interface than returning only the first text content. It sacrifices consistency with the other accessors; however, text *is* different from the others including having empty string as a good substitute for None, and having a natural way to concatenate the parts. Also, update __iter__ docstring to reflect that we no longer have Text parts distinct from str.
The variable return type of content (Content | Sequence[Content]) was inconsistent with all the other accessors, and it prevented using response.content as an iterator. If response.content is always a sequence, then there's no need for response to be an iterator - you can just do `for item in response.content` instead.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
v2-docs | d1c828d | Commit Preview URL | Jul 03 2025, 12:09 AM |
willbakst
reviewed
Jul 2, 2025
This reverts commit a570d3f.
willbakst
reviewed
Jul 2, 2025
willbakst
approved these changes
Jul 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Major changes in this PR:
"openai:gpt-4o-mini") for consistency with the call interfaceRationale:
It's a bit confusing to center "BaseResponse" as the type that you get back from calls — much nicer to say you get a
llm.Response. However, that is inaccurate unless we change the class names. Note that unless you are worried about tools, you don't actually care about SimpleResponse vs ContextResponse - and in that case you can refine the type you are assigning to.Simplify ResponseContent type implementation while maintaining type safety, differentiate SimpleResponseContent and ContextResponseContent
Modify
response.textso that it gives back all text content, concatenated, and an empty string if empty. This makes it slightly less consistent with the other content-type accessors, however text is already special in multiple respects: in being a raw string rather than a dataclass, and in having a very natural empty representation ("") and a very natural way to concatenate multiple text parts. IMO having it return the first text piece (but potentially not all of the text) is leaking an implementation detail in a way that will confuse users.Make
response.contentalways a sequence of content. This is simpler and more consistent with the other accessors.Make Response no longer an iterable. There is no need when you can just iterate over
response.content.Add thinking content to responses.
Add unified exception classes, rather than passing through provider-specific exceptions. This is a much better fit for Mirascope v2 being unified and holistically abstracting over the provider APIs.
Add an initial draft of calls.mdx
Enable custom clients, add to docs
Enable params (moving temperature and max_tokens to generic params across all providers), add to docs
Another note: I wrote the intro content for calls.mdx to show the spec and functional style, and then wrote the subsequent examples in the functional style. I propose this as a pattern going forward for the coming guides. I think this strikes a nice balance between consistently showing the users that both styles are available and they can choose their preference, and not needing to duplicate every single example, particularly ones where there is no added educational value in showing both functional and spec styles. Even if it doesn't clutter the user view, it saves maintenance burden for us, and it reduces wasted tokens in the LLM text (the LLM does not need to see every single example in both styles)