feat: Record and replay Turn objects and its dependencies #503
Conversation
Need to remove with_chat and instead use as argument. Avoids temp global state state / clearly provides context
* main: (26 commits) Drop `completed` (tidyverse#495) Add PortkeyAI support (tidyverse#471) feat: Adds `stream = "content"` option to `$stream()` and `$stream_async()` (tidyverse#494) feat(Chat): `tool_request` and `tool_result` callbacks (tidyverse#493) feat: Add `tool_reject()` (tidyverse#490) fix: Tweak simple tools test to avoid Claude's empty strings (tidyverse#492) feat(chat_async): Adds `tool_mode` to choose between sequential or concurrent tool calling (tidyverse#488) And azure :| Update snapshot tests; fix bad bedrock model choice chore: Remove trailing whitespace from NEWS (tidyverse#489) Add (some) functions for listing models (tidyverse#454) Move `chat$extract_data_parallel()` to `parallel_chat_structured()` (tidyverse#486) refactor: Use generators for tool invocation (tidyverse#487) refactor: Remove private invoke_tools methods (tidyverse#485) Automatically convert tool inputs (tidyverse#463) Allow `$extract_data_parallel()` to return tokens + cost (tidyverse#449) Start work on programming vignette (tidyverse#458) Revise type coercion (tidyverse#484) Tweak `chat_openai_test()` (tidyverse#483) refactor(tools): `match_tools()` and `tool_results_as_turn()` (tidyverse#480) ...
…olDef if one does not exist
schloerke
force-pushed
the
turns_record_replay
branch
from
c4b2b03 to
ae4dc7e
Compare
R/content-replay.R
Outdated
| #' basic list that can be easily serialized. | ||
| #' * `contents_replay()` will accept a basic list (from `contents_record()`) and | ||
| #' return a corresponding [Turn] or [Content] related object. | ||
| #' * `contents_replay_class()` is a generic function that is dispatched from |
There was a problem hiding this comment.
A lot of the remaining comments come down to generalized support...
Should we allow for external package support? Ex: A tool call that returns an S7 object from a package outside ellmer.
The current implementation is set up for multiple package support (which would like contents_replay_class() to be exported for external package customization).
Another nice-ity that all props are auto inspected for S7 values. By default, if they are an S7 value or an unnamed list of S7 values, the value is recorded as well.
If we want to manually handle contents_record() for every ellmer classes only, then yes everything becomes simpler in implementation and we should error for contents_replay() values that are not replay values.
There was a problem hiding this comment.
Yeah, I think we should just tightly scope to ellmer for now. We can consider expanding support if/when needed.
Co-Authored-By: Hadley Wickham <[email protected]>
Also remove now _difficult to test_ "returns non-s7 object" from `contents_replay()`
To be used for bookmarking
{ellmer}clients within{shinychat}(posit-dev/shinychat#28)Fixes #502
Provides three methods:
contents_record(content, ..., chat)- Generic that will convert S7 objects into jsonifiable objectscontents_replay(obj, ..., chat)- Normal R method that takes the jsonifiable objects and recreate an S7 objectcontents_replay_class(cls, obj, ..., chat)- Generic that will dispatch on the class object saved fromcontents_record().clswill not typically be utilized in custom implementations, it is useful for restoring in the base situation.clsis needed for dispatch. Otherwise allobjvalues are named lists.The
chatobject is required to be passed down in all methods to provide context without using global state.The current test goes through the shiny bookmarking experience:
TODO: