-
Notifications
You must be signed in to change notification settings - Fork 0
reconstruct tool calls #30
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,8 +3,8 @@ | |
| # AUTOGENERATED! DO NOT EDIT! File to edit: ../nbs/00_core.ipynb. | ||
|
|
||
| # %% auto 0 | ||
| __all__ = ['sonn45', 'effort', 'patch_litellm', 'mk_msg', 'mk_msgs', 'stream_with_complete', 'lite_mk_func', 'cite_footnote', | ||
| 'cite_footnotes', 'Chat', 'random_tool_id', 'mk_tc', 'mk_tc_req', 'mk_tc_result', 'mk_tc_results', | ||
| __all__ = ['sonn45', 'effort', 'patch_litellm', 'mk_msg', 'stream_with_complete', 'lite_mk_func', 'cite_footnote', | ||
| 'cite_footnotes', 'Chat', 'random_tool_id', 'mk_tc', 'mk_tc_req', 'mk_tc_result', 'mk_tc_results', 'mk_msgs', | ||
| 'astream_with_complete', 'AsyncChat', 'aformat_stream', 'adisplay_stream'] | ||
|
|
||
| # %% ../nbs/00_core.ipynb | ||
|
|
@@ -117,23 +117,6 @@ def mk_msg(content, # Content: str, bytes (image), list of mixed content, o | |
| msg = {"role": role, "content": c} | ||
| return _add_cache_control(msg, ttl=ttl) if cache else msg | ||
|
|
||
| # %% ../nbs/00_core.ipynb | ||
| def mk_msgs(msgs, # List of messages (each: str, bytes, list, or dict w 'role' and 'content' fields) | ||
| cache=False, # Enable Anthropic caching | ||
| ttl=None, # Cache TTL: '5m' (default) or '1h' | ||
| ): | ||
| "Create a list of LiteLLM compatible messages." | ||
| if not msgs: return [] | ||
| if not isinstance(msgs, list): msgs = [msgs] | ||
| res,role = [],'user' | ||
| for m in msgs: | ||
| res.append(msg:=mk_msg(m, role=role)) | ||
| role = 'assistant' if msg['role'] in ('user','function', 'tool') else 'user' | ||
| if cache: | ||
| res[-1] = _add_cache_control(res[-1], ttl) | ||
| res[-2] = _add_cache_control(res[-2], ttl) | ||
| return res | ||
|
|
||
| # %% ../nbs/00_core.ipynb | ||
| def stream_with_complete(gen, postproc=noop): | ||
| "Extend streaming response chunks with the complete response" | ||
|
|
@@ -269,11 +252,9 @@ def random_tool_id(): | |
| return f'toolu_{random_part}' | ||
|
|
||
| # %% ../nbs/00_core.ipynb | ||
| def mk_tc(func, idx=1, **kwargs): | ||
| args = json.dumps(kwargs) | ||
| if callable(func): func = func.__name__ | ||
| id = random_tool_id() | ||
| return {'index': idx, 'function': {'arguments': args, 'name': func}, 'id': id, 'type': 'function'} | ||
| def mk_tc(func, args, tcid=None, idx=1): | ||
| if not tcid: tcid = random_tool_id() | ||
| return {'index': idx, 'function': {'arguments': args, 'name': func}, 'id': tcid, 'type': 'function'} | ||
|
Comment on lines
+255
to
+257
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These changes make it easy to create tool messages from the tool call summary message (i.e. when the called function and args are strings). The downside is that there's a little more effort involved in creating a tool call message when the function and args are symbols. For example. Here's the syntax on main mk_tc(simple_add, a=5, b=7)vs the syntax for this pr. mk_tc(simple_add.__name__, json.dumps(dict(a=5, b=7))) |
||
|
|
||
| # %% ../nbs/00_core.ipynb | ||
| def mk_tc_req(content, tcs): | ||
|
|
@@ -287,6 +268,48 @@ def mk_tc_result(tc, result): return {'tool_call_id': tc['id'], 'role': 'tool', | |
| # %% ../nbs/00_core.ipynb | ||
| def mk_tc_results(tcq, results): return [mk_tc_result(a,b) for a,b in zip(tcq.tool_calls, results)] | ||
|
|
||
| # %% ../nbs/00_core.ipynb | ||
| def _details_extract(x): | ||
| "Extract fn, args, tool_call_id, result from <details>" | ||
| m = re.search(r'<details.*?>(.*?)</details>', x, re.DOTALL) | ||
| tc, tcid, res = re.findall(r'-\s*`([^`]+)`', m.group(1)) | ||
| fn, args = re.search(r'(\w+)\((.*?)\)', tc).groups() | ||
| return fn, args, res, tcid | ||
|
Comment on lines
+272
to
+277
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This pr modifies the tool call summary message by including a tool call id in the details section. As a result, That version generates a message like this I'll use the `addy` function to add 5 and 3 for you.
<details class='tool-usage-details'>
`addy({"a": 5, "b": 3})`
- `8`
</details>
The result is 8.Whereas this pr expects this structure I'll use the `addy` function to sum 5 and 7 for you.
<details class='tool-usage-details'>
- `simple_add({"a": 5, "b": 7})`
- `12`
- `toolu_01RPbSeouj8mc2N4rfjw2BaH`
</details>
The sum of 5 and 7 is **12**.We could make it backwards compatible by using a random tool call id? Maybe this is overkill? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using regexen here doesn't feel robust to me. I'd have thought that using json would be better - i.e. a proper structured format with a well-tested standard implementation. wdyt? |
||
|
|
||
| # %% ../nbs/00_core.ipynb | ||
| def _build_tool_hist(msg): | ||
| "Build original tool call messages from the tool call summary." | ||
| hist = [] | ||
| parts = re.split(r'(<details.*?>.*?</details>)', msg, flags=re.DOTALL) | ||
| for islast, (i,o) in loop_last(enumerate(parts)): | ||
| if "<details class='tool-usage-details'>" not in o: | ||
| if islast: hist.append(o.strip()) | ||
| continue | ||
| fn, args, res, tcid = _details_extract(o) | ||
| tc = mk_tc(fn, args, tcid) | ||
| tcq = mk_tc_req(parts[i-1].strip() if i>0 else "", [tc]) | ||
| tcr = first(mk_tc_results(tcq, [res])) | ||
| hist.extend([tcq, tcr]) | ||
| return hist | ||
|
|
||
| # %% ../nbs/00_core.ipynb | ||
| def mk_msgs(msgs, # List of messages (each: str, bytes, list, or dict w 'role' and 'content' fields) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Redefining There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah the implementation feels a bit over-clever to me. It needn't be so integrated and automatic. It's quite special-case behaviour. Instead, I'd expect to have a function like "extract_tcs()" which you pass a message to, and it turns it into a list of messages with tool calls expanded. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We went back and forth on this. We opted for the automatic implementation because we didn't see a strong reason not to expand these messages. For |
||
| cache=False, # Enable Anthropic caching | ||
| ttl=None, # Cache TTL: '5m' (default) or '1h' | ||
| ): | ||
| "Create a list of LiteLLM compatible messages." | ||
| if not msgs: return [] | ||
| if not isinstance(msgs, list): msgs = [msgs] | ||
| res,role = [],'user' | ||
| msgs = L(msgs).map(lambda m: _build_tool_hist(m) if "<details class='tool-usage-details'>" in m else [m]).concat() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line ensures that we automatically reconstruct the tool call history for every tool call summary msg in msgs. |
||
| for m in msgs: | ||
| res.append(msg:=mk_msg(m, role=role)) | ||
| role = 'assistant' if msg['role'] in ('user','function', 'tool') else 'user' | ||
| if cache: | ||
| res[-1] = _add_cache_control(res[-1], ttl) | ||
| res[-2] = _add_cache_control(res[-2], ttl) | ||
| return res | ||
|
|
||
| # %% ../nbs/00_core.ipynb | ||
| async def _alite_call_func(tc, ns, raise_on_err=True): | ||
| try: fargs = json.loads(tc.function.arguments) | ||
|
|
@@ -385,9 +408,9 @@ async def aformat_stream(rs, include_usage=False): | |
| if include_usage: yield f"\nUsage: {o.usage}" | ||
| if (c := getattr(o.choices[0].message, 'tool_calls', None)): | ||
| fn = first(c).function | ||
| yield f"\n<details class='tool-usage-details'>\n\n `{fn.name}({_trunc_str(fn.arguments)})`\n" | ||
| yield f"\n<details class='tool-usage-details'>\n\n - `{fn.name}({_trunc_str(fn.arguments, replace='<TRUNCATED>')})`\n" | ||
| elif isinstance(o, dict) and 'tool_call_id' in o: | ||
| yield f" - `{_trunc_str(_clean_str(o.get('content')))}`\n\n</details>\n\n" | ||
| yield f" - `{o['tool_call_id']}`\n\n - `{_trunc_str(_clean_str(o.get('content')),replace='<TRUNCATED>')}`\n\n</details>\n\n" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We needed to include the |
||
|
|
||
| # %% ../nbs/00_core.ipynb | ||
| async def adisplay_stream(rs): | ||
|
|
||
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.
Although these 2 items are unrelated to the pr, they were automatically added to the cache because their
chathistory is linked to upstream calls. This dependency has been removed in this pr.