Explicit Tool Turns

[psionic-k]

Draft

And does not pass tests yet. Should be close to passing. Just sleepy and wanted to give PST a chance to take a look at approaches before I finish up.

One of my commits almost vanished during merge and became whitespace only.

• tool-call & tool-result correspondence enforcement & warning
• make tests pass
• tool call display function name & args via overlay, re-hydrate overlays when opening file

[karthink]

The general approach looks good to me. I one major suggestion and one question. The suggestion first:

I don't think it's worth propertizing the prompt-prefix and response-prefix. The current approach is to strip them via text search (see gptel--trim-prefixes). Additionally, all leading and trailing whitespace is stripped from the user prompt as well, so the most common gptel-response-separators are taken care of automatically.

I understand this is less than ideal when you change the prefixes. For example, if you open an old chat file from before you changed the prefixes, the prefixes in the file will no longer be stripped.

However, I think the new approach is worse on balance because it doubles or triples the length of the serialized bounds list when you save the chat. We want to store as little metadata as we can get away with. Note also that the local variables block which is used for serialization in markdown and text chat files has a maximum allowed length of 2000 characters or something. (Enforced by Emacs.) Each extra character of metadata we store bumps us closer to this limit, and all hell breaks loose if we cross it.

Continuing to strip prefixes/suffixes using string matching will also simplify the code a fair bit -- no need for a gptel-response-separator function, for instance.

Finally, I haven't noticed any auto-mimicry problems with retaining the prefixes in the messages array, possibly because if the prefixes fail to be stripped, they always end up as part of the user prompts, not previous LLM responses. So failing to get it right is not a critical problem.

To address the case of rehydrating older chat files correctly, if we absolutely must strip the prefixes correctly we can store the prefix strings in the file as Org properties/local variables. I'm not sure this is required, and in any case it doesn't have to be part of this PR.

Will address the question in my next message.

[karthink]

IIUC, you haven't yet updated gptel--get-buffer-bounds, is that correct? This is required for serializing the response (and now tool result) bounds to the buffer. When you implement this, keep in mind that we want to be as parsimonious as possible. So response text bounds (the most common case) should still be something like (1234 1500). Currently, you are assuming here that it will look like (1234 1500 response). The ignore case can also probably be made shorter/simpler than (1612 1700 ignore).

[karthink]

See my comment about using (beg end) instead of (beg end response).

[karthink]

Can be made simpler/shorter somehow, perhaps?

[karthink]

The gptel text-property is globally rear-nonsticky (see text-property-default-nonsticky) so this is not required.

[karthink]

This changes the tool-use API, and also makes the check at line 2732 fail... I think? Need to take a closer look.

[karthink]

The tool-use API looks like this right now: the gptel-request callback is called with two different types of lists for tool calls and tool results as the RESPONSE argument.

For tool calls, it is called with a list of the form

((tool args closure) ...)

where tool is a gptel-tool struct. (The closure here is the closure used to continue the gptel request with the tool call result.)

For tool results, it is called with a list of the form

((name args result) ...)

where name is a string, the name of the tool.

• These have to be of different types because the callback needs to know if what it received was a tool call (waiting for confirmation/run) or a tool result (to insert into the buffer etc).

• Further, these have to be different in a way that's easy for a package author to understand, since this is part of the gptel-request API.

• I judged the more common case to be the one involving the tool result, which is why it's simpler -- an alist mapping a tool name (like "get_buffer") to the LLM-supplied args and result of calling it. In contrast, the ((tool args callback) ...) case sends the callback the tool object itself since it needs to run the tool function (gptel-tool-function tool).

• Bonus: it would be good to hew close to the API of the llm package, for future cooperation/coordination/reuse. As I understand it, llm package does not currently do the first kind of call at all (no tool call confirmations), so it has an easier time of it. It sends ((name . result) ...) to the callback.

For your use case, this API needs to be redesigned because you need the tool call id along with the result. We should keep the above goals in mind when making the API change.

---

One option I considered before is an explicit mention of the purpose of the call, i.e. send to the gptel-request callback

(:tool-call ((tool args closure) ...))

for a tool-call confirmation/run, and

(:tool-result ((tool args result) ...))

for a tool result.

I judged this as an unnecessarily complex structure before (a plist of list of lists). Same for

(tool-call   . ((tool args closure)...)) ;and
(tool-result . ((tool args result) ...))

These are too verbose because most consumers of gptel-request + tools will only ever see one of these, if at all. I prefer the simplicity of ((name args result) ...) if we can get away with it.

See the documentation for other types of REPONSE gptel-request can send to the callback:

• a string for response text,
• nil if there's an error,
• the symbol abort if the request was aborted.

We want something at this level of simplicity for tool calls and tool results.

[karthink]

We also need to add new tests, but maybe that can wait until the design is final?

[psionic-k]

Note also that the local variables block which is used for serialization in markdown and text chat files has a maximum allowed length of 2000 characters or something.. ..all hell breaks loose if we cross it

Technically it's broken for big chats already. When the day comes that someone needs this, we can use file storage. Add an option for the property to be a string. The string names a file at (in the no-littering case) /user-emacs-dir/var/gptel/bounds/<UUID-or-hash>.el. No longer self-contained, but IMO unavoidable in the end ✂️.

I understand this is less than ideal when you change the prefixes.

It's not something I would stress over. I agree stripping can be very valuable in a few cases. An empty line can be ignored as a user message instead of treated blindly like a turn in the chat. In that case, I no longer need to mark some lines as ignore. The ignore property can be re-hydrated from #+tool_call and ```tool-call matches almost as easily as it can be stored.

The tool-use API

Aight, I think I will have to asynchronously answer this part.

[karthink]

When the day comes that someone needs this, we can use file storage. Add an option for the property to be a string. The string names a file at (in the no-littering case) /user-emacs-dir/var/gptel/bounds/.el. No longer self-contained, but IMO unavoidable in the end.

Losing self-contained files is not acceptable. At that point we can switch to using a TOML frontmatter block or something in Markdown files instead, the equivalent of Org properties. But anyway, this is not currently a problem.

It's not something I would stress over. I agree stripping can be very valuable in a few cases. An empty line can be ignored as a use-message instead of treated blindly like a turn in the chat. In that case, I no longer need to mark some lines as ignore. The ignore property can be re-hydrated from #+tool_call and ```tool-call matches almost as easily as it can be stored.

Not sure what you mean by "stress over it". The current approach of stripping the prefixes via text search is simple and sometimes wrong, but that's me not stressing over it.

I'd like to avoid explicitly marking prefixes as ignored text for the mentioned reasons:

1. it's okay to get this wrong sometimes,
2. it avoids extra code in gptel,
3. it doubles/triples the length of the serialized bounds, and
4. it's easy to add in the future, but not as easy to remove.

Adding the ignore property to the tool call block decorations is fine 👍

[karthink]

An empty line can be ignored as a user-message instead of treated blindly like a turn in the chat.

Empty/whitespace lines should already be stripped, IIRC.

[psionic-k]

Not sure what you mean by "stress over it". The current approach of stripping the prefixes via text search is simple and sometimes wrong, but that's me not stressing over it.

I meant to confirm that not stressing over it was already correct. My reasoning was that any use case for keeping prefixes in sync is extremely niche and can be done by simple text replace if the user really needs.

[karthink]

The ignore property can be re-hydrated from #+tool_call and ```tool-call matches almost as easily as it can be stored.

If I'm reading this right, you're debating if you should serialize the bounds of #+tool_call and ```tool-call decorators as (beg end ignore) when writing the buffer to disk, and instead use text search to mark these as ignored when restoring the chat state?

[psionic-k]

The ignore property can be re-hydrated from #+tool_call and ```tool-call matches almost as easily as it can be stored.

If I'm reading this right, you're debating if you should serialize the bounds of #+tool_call and ```tool-call decorators as (beg end ignore) when writing the buffer to disk, and instead use text search to mark these as ignored when restoring the chat state?

Avoid serialization, yes. However, instead of marking them ignore, just stripping them out before calling gptel--parse-buffer.

I think I can filter them on the frontends easy enough. That seems more proper since they're an artifact of each frontend anyway. In both markdown and org, stripping these block header + footers is just a linear scan going forward if we escape the block contents. It's more robust in the case that someone munges a block. ignore is still really useful, but probably should be avoided where stripping is trivial.

[karthink]

Avoid serialization, yes.

Sure. As I see it the less we need to serialize the more robust the persistence feature remains.

However, instead of marking them ignore, just stripping them out before calling gptel--parse-buffer.

Hmm, this requires going all-in into using a buffer copy for parsing. We're currently doing this for Org because of gptel-org-branching-context and org-unescape-code-in-region, but still avoiding it in non-Org buffers. This is more than just Markdown -- it's every other major-mode in which gptel is used.

We've paid the price for Org already, but I'm hoping to avoid doing this everywhere else.

I think I can filter them on the frontends easy enough. That seems more proper since they're an artifact of each frontend anyway. In both markdown and org, stripping these block header + footers is just a linear scan going forward if we escape the block contents.

Well one of your original arguments was that using intervals (text-properties/overlays) is both more efficient (O(1) or O(log(point-max)) or something) and robust, and we don't pay a parsing penalty. I like the idea.

It's more robust in the case that someone munges a block.

If someone messes up a block, aren't text properties more robust?

If you're using #+begin_tool_call... and someone messes this up to read #+begin_tool_cal..., it will still work fine if you're using the ignore property. The same argument applies to messing up the contents of the block. The tool call args or results as recorded will be distorted, but it will still be recognized as a tool call.

On the other hand, if you're text-matching #+begin_tool_call and it's modified, it will get inserted as part of the response, leading eventually to auto-mimicry.

ignore is still really useful, but probably should be avoided where stripping is trivial.

I propose a hybrid solution:

1. Don't serialize the tool call block decorators as (beg end ignore). This keeps the serialized list short and tidy.
2. When rehydrating, use text matching and do apply the ignore property to the decorators. This requires text processing, which is slower, but it only runs once on rehydration instead of before every request.
3. When creating the payload, respect the current ignore rules. This way you don't need to create a new buffer to filter out text except in Org mode (where it is required for other reasons.)

[psionic-k]

We've paid the price for Org already, but I'm hoping to avoid doing this everywhere else.

If we're talking about 100MB contents, I could see this being heavy, but especially where we don't even activate the mode, buffer copying is idiomatic rather than anti-pattern. It's super cheap and in many cases much more straight-forward. We have like 5GB/s available on this kind of workload.

Try this function out on a typical buffer:

(defun clone-and-report ()
  (interactive)
  (let ((source (current-buffer))
        (beg (point-min))
        (end (point-min))        
        (conses cons-cells-consed)
        (start (float-time))
        (created))
    (dotimes (_ 100)
      (let ((buffer (generate-new-buffer (buffer-name) t)))
        (set-buffer buffer)
        (insert-buffer-substring source beg end)
        (push buffer created)))
    (let ((new-conses cons-cells-consed)
          (now (float-time)))
      (message "Old: %s New: %s Delta: %s"
               conses new-conses (- new-conses conses))
      (message "Start: %s End: %s Elapsed: %s"
               start now (- now start)))
    (mapcar #'kill-buffer created)
    (let ((new-conses cons-cells-consed)
          (now (float-time)))
      (message "After killing - Old: %s New: %s Delta: %s"
               conses new-conses (- new-conses conses))
      (message "After killing - Start: %s End: %s Elapsed: %s"
               start now (- now start)))))

I ran this on one of my org buffers and it was 4ms and 1858 conses, many of which are used to run the command and print the results. It's way, way faster than human speed and doesn't gum up the GC.

There's reasons to do other things like only copying a region from the buffer, but in general buffer-to-buffer copy is one of the fastest, cheapest, lowest reside things in Emacs.

[karthink]

There's reasons to do other things like only copying a region from the buffer, but in general buffer-to-buffer copy is one of the fastest, cheapest, lowest reside things in Emacs.

I take your point. 👍

Independent of this, did you think about the other arguments (robustness to text munging, interval trees vs string/regexp search) for painting the tool call block decorators with the ignore property and avoiding text parsing?

[psionic-k]

If someone messes up a block, aren't text properties more robust?

Unless we save them, until restoring. Hmm...

Btw, linear scan are O(n). Grows with buffer size. Parsing without the need to backtrack is O(n) in time. Realistically it's only heavy in org because of the overhead of push-down and deciding and scooping up every single kind of element into the result.

((name . result) ...)

Without the id of the tool call, the API is incomplete. Possibly the llm package is incomplete and needs to make a breaking change here. Without ids, I'd say it's broken to begin with.

I'll look around, but I definitely like the idea of any state machine callback receiving an explicit indication of state, either through a method name or a leading symbol.

(:tool-call . ((tool args))) and (:tool-result . ((tool-result args result))) are my instinct. I need to look at the other callbacks. I don't like implicit information when matching, such as "if it's a cons, do Y" because that kind of stuff is made to break.

Whatever the case, since the feature is young, I'd be in favor of an aggressive breaking change to get to a good place early in its life, one that is easy to understand. Let's not make it backwards compatible but convoluted.

[psionic-k]

After looking through a bit more, I don't think any :tool-call or :tool-result is necessary here. The type check of the caar is fine for now. In the default callback, both results are lists and so are passed correctly to gptel--display-tool-calls.

It wouldn't be the worst idea to add a TYPE argument to the callback but I just don't think it solves enough here. The user's callback is about as simple as it can get unless a separate :tool-callback is made to separate the two entirely.

I need to throw some time at this tomorrow. Too sleepy to write anything smart.

[karthink]

Unless we save them, until restoring. Hmm...

I don't follow. I meant that painting the decorators with ignore is more robust against fat-fingering by the user.

Btw, linear scan are O(n). Grows with buffer size. Parsing without the need to backtrack is O(n) in time. Realistically it's only heavy in org because of the overhead of push-down and deciding and scooping up every single kind of element into the result.

Interval tree lookups are O(log(m)), where m is the number of intervals (as opposed to buffer-size in chars), and the proportionality constant and memory requirements are much lower too. regex search is a heavy operation in comparison. That said, you're right that practically it won't matter until org-element enters the picture.

Without the id of the tool call, the API is incomplete. Possibly the llm package is incomplete and needs to make a breaking change here. Without ids, I'd say it's broken to begin with.

I disagree, the tool call id is an internal communication detail. There's no reason the callback/API consumer needs to be aware of some random UUID. Your case is special since you want to encode the messages array as the buffer in a lossless way. (See my note at the end.)

Let's not make it backwards compatible but convoluted.

The concern isn't backwards compatibility, as there is no "backwards" yet. We are at the start line.

The concern is the complexity of and confusion in the gptel-request API. Right now you're sending the callback these structures:

For tool calls confirmation/run:

((#[gptel-tool name function args description category ...] args closure) ...)

where #[...] is the gptel-tool struct.

For tool results:

((:name ... :args ... :result ... :id ... :arguments ... :function ...) ...)

Except for closure in the first case and :result in the second, these contain the exact same information packaged two different ways, both of which are quite complex. This seems convoluted to me. And possibly very confusing for a gptel-request consumer.

(The structure of the plist above isn't even uniform across backends, by the way. For OpenAI the above has both :args and :arguments, for example. However, all of them will have the keys :name, :args, :function, which are the only ones we need.)

Compare with

((#[gptel-tool name function args description category ...] args closure) ...)

for a tool call confirmation/run and

((name args result) ...)

for a tool result, keeping in mind that the latter is going to be the much more common in uses of gptel-request.

These two are much more distinct.

I don't like implicit information when matching, such as "if it's a cons, do Y" because that kind of stuff is made to break.

But this is what's happening right now.

---

In any case, the user can switch backends at any time so whatever tool id you store has to work for all backends. So I have a hypothesis: modulo some template or prefix like "call_", the tool call id does not matter as long as it's unique. If this is true we don't need to pass the tool call id to the callback at all, you can just make up a unique identifier.

[psionic-k]

The concern isn't backwards compatibility, as there is no "backwards" yet. We are at the start line.

agree.

This seems convoluted to me. And possibly very confusing for a gptel-request consumer.

The structure of the plist above isn't even uniform across backends, by the way.

we don't need to pass the tool call id to the callback at all

I snipped a great deal of distracting reading because I need more answers from reading / writing code. Here is another great deal of less distracting reading. 🥲

While not for now, I think backends will need to be able to leak data into the front-end, but in a completely pass-through manner, using a gptel-<backend>-<model> property. Backend context caching is such a detail.

I think it would be better to always pass tool structs to the callback rather than "tool-name". This can later support a :passthrough or :backend slot on the tool struct that is then blindly attached to the buffer contents through properties without attempting to decipher or manipulate them. Passing tool structs is more consistent.

The calls & results would be: (tool args closure) and (tool args result).

In this case, while the id is hopefully not backend specific, I would like to use it and just hope for the best. If it doesn't matter, why replace it with another unique value unless we know such a value will fail less and not more? How this can actually help backends (and they might not tell us) is that the computation up to the tool call might be cached, leading to faster first tokens. All of the big providers are strongly incentivized to silently cache and use several methods of content cleaning and hashing to avoid re-computing the model state up to the next token.

I does seem better to provide the callback with RESULT INFO TYPE so that the user can just pcase on TYPE instead of having to look at the type of a caar in a list etc. That's the kind of decision that is inconvenient and easy to get wrong in the callback. If the user misses a case, they can warn / log on _, so it's robust moving forward.

Your case is special

Which part, the round tripping of the backend detail or retaining the tool result in context?

I meant that painting the decorators with ignore is more robust against fat-fingering by the user.

If during restore I'm parsing decorators to re-ignore them, the fat fingered decorator will not be propertized. Only if we save the properties of ignored content does this advantage hold during persistence.

There's other problems with fat fingering, such as leaving behind a result that can no longer be read. Maybe we should mark every tool call as read-only and be done with these user what-if headaches. It's almost never meaningful for them to update the results by hand unless they just delete the entire tool call. I'm going with this.

[karthink]

While not for now, I think backends will need to be able to leak data into the front-end, but in a completely pass-through manner, using a gptel-<backend>-<model> property. Backend context caching is such a detail.

I'm not following, but I'll wait for the PR to develop further instead of taking up more of your time on future plans right now.

I think it would be better to always pass tool structs to the callback rather than "tool-name". This can later support a :passthrough or :backend slot on the tool struct that is then blindly attached to the buffer contents through properties without attempting to decipher or manipulate them. Passing tool structs is more consistent.

• Always passing tool structs: ✅ I like the uniformity/simplicity of this.
• Adding a slot to the tool struct to pass information to the buffer ❌ Hard no. This is a mingling of unrelated concerns. The tool struct is pretty much an immutable object. It should not be responsible for passing data from the "backend" to the "frontend", as you put it.

In this case, while the id is hopefully not backend specific, I would like to use it and just hope for the best. If it doesn't matter, why replace it with another unique value unless we know such a value will fail less and not more?

You still need a whole system written around the tool call ids to accommodate the various APIs. If you've stored call_1234567890 and the user now switches to Anthropic, you have to convert it to toolu_1234567890. It's better to use a common internal representation and store that instead, pushing the id templating logic into the parser functions for each backend.

How this can actually help backends (and they might not tell us) is that the computation up to the tool call might be cached, leading to faster first tokens. All of the big providers are strongly incentivized to silently cache and use several methods of content cleaning and hashing to avoid re-computing the model state up to the next token.

I understand the idea in general, but doubt very much tool call ids have anything to do with it. It also depends on where the cache boundary is placed. Tool calls appear in the last two messages in the messages array, unlikely to make much difference.

The calls & results would be: (tool args closure) and (tool args result).

I like this and agree, except for two things:

1. Neither of these involves passing the tool call id, which you want
2. Your following point about distinguishing between them won't work:

I does seem better to provide the callback with RESULT INFO TYPE so that the user can just pcase on TYPE instead of having to look at the type of a caar in a list etc. That's the kind of decision that is inconvenient and easy to get wrong in the callback. If the user misses a case, they can warn / log on _, so it's robust moving forward.

Changing the callback calling convention is a no-no. This would make it a backwards compatibility issue.

But it's also unnecessary, since INFO is a bucket meant for exactly this kind of dynamic information and variadic use. Instead of TYPE it can be stored as (plist-put INFO :type 'tool-call) or something, and the callback can look it up if RESPONSE is a cons. Other uses of the callback remain unaffected.

However, looking at integrating the Deepseek model properly is convincing me that the callback is going to have to fulfill an increasing number of duties. In this case, it has to decide how to handle the "reasoning" text block supplied by Deepseek, for which gptel needs to signal that the provided text is a reasoning block. So a dispatch system where RESPONSE is set to different lists is looking more appealing:

tool-call:

(tool-call . ((tool args closure) ...)) ; or just (call . ((...)))?

tool-result:

(tool-result . ((tool args result) ...)) ; or just (result . ((...)))?

reasoning block:

(reasoning . "The user is asking for...")

This is an alternative to running (plist-get info :type 'tool-result) before sending the callback the results, etc.

Your case is special

Which part, the round tripping of the backend detail or retaining the tool result in context?

The latter, you're trying to map the messages array to the buffer in a one-to-one fashion. I want this for gptel's chat feature, but most other uses of tools don't have this goal.

[karthink]

I meant that painting the decorators with ignore is more robust against fat-fingering by the user.

If during restore I'm parsing decorators to re-ignore them, the fat fingered decorator will not be propertized. Only if we save the properties of ignored content does this advantage hold during persistence.

Yes, you lose robustness the moment text parsing enters the picture. But it's not all or nothing, as many chats (I'm guessing a majority) are single-use and not persisted to disk, and it works fine for these.

Alternatively, you can save the properties of the ignored content too, if we can find some way to shorten the length of the serialized string: perhaps just (241 276 i) instead of (241 276 ignore).

On that note, tool calls and result properties can also be shortened when serialized, to tc and tr?

(I must reiterate here that the prompt/response prefix don't deserve this much care as they're part of the user prompt and not a cause of auto-mimicry. Even applying ignore is unnecessary, text parsing should continue to work fine for them.)

There's other problems with fat fingering, such as leaving behind a result that can no longer be read.

I'm imagining the situation where a result is split into two chunks by an errant yank. Or worse, when a tool call (name, args) is split.

There needs to be some way to sanitize a parsed tool block. I see a TODO item in your PR about coupling tool calls and results so they appear next to each other in the messages array.

In the worst case it should all be parsed and included as regular response text, I guess.

Maybe we should mark every tool call as read-only and be done with these user what-if headaches. It's almost never meaningful for them to update the results by hand unless they just delete the entire tool call. I'm going with this.

You can't mark a region of the chat buffer read-only if it keeps the user from running, say, M-< C-w. On the other hand, ensuring that deletion is allowed when it's atomic across the whole tool result block is a fair compromise.

But how do you ensure that the entire tool call can be deleted but a part of it can't? I can think of a couple ways to do this; they're super clunky.

[psionic-k]

I want this for gptel's chat feature, but most other uses of tools don't have this goal.

They will. The front-ends are just rudimentary right now. The killer pattern will be having a usually hidden but editable session buffer to store and remix context. When re-writing regions, you want gptel to have seen how other source code works and to remember it for the duration of potentially 2-3 re-write attempts with different prompts. This session buffer can be used across multiple buffers, such as with occur workflows. It can be cloned to start multiple requests in parallel, forked, supports undo barriers and so on. Chat combines the idea of a working buffer and a target buffer.

It's kind of like macros with LLMs providing the gummy heuristics and pseudo-programming interface and tools providing the RAG data necessary for contextualizing work with accurate facts.

But it's also unnecessary, since INFO is a bucket meant for exactly this kind of dynamic information and variadic use.

I'm hesitant because it's a shared mutable object that lives for the entire request lifecycle. Doesn't feel right to throw such a clear function signature relationship in there.

I like your later suggestion of 'tool-call in the car better.

So a dispatch system where RESPONSE is set to different lists is looking more appealing

Not the best, but simple enough. Recommend putting a TODO(2.0) on this?

Neither of these involves passing the tool call id, which you want

Ah, hell. True. I was off on a tangent imagining there was a tool-call object.

If you've stored call_1234567890 and the user now switches to Anthropic, you have to convert it to toolu_1234567890. It's better to use a common internal representation

• template & de-template 👍
• made up ID 🙅

Recommend:

(tool-call ((tool args closure id))...) and (tool-result ((tool args result id))...). Backends without ids can generate it.

Tool calls appear in the last two messages in the messages array, unlikely to make much difference.

The opposite is true. As I've demonstrated, tool calls are going to wind up heavily interwoven in recursive lookup use cases. We're pretty good at sending an append-only log in many cases. It's good to be consistent.

On that note, tool calls and result properties can also be shortened when serialized, to tc and tr?

We can even combine tool calls and results. It might make other problems simpler. I'll take a look.

A 2D structure is most efficient here. For enough flexibility in the values:

(response (beg end) (beg end)
 tool (beg end id) (beg end id)
 ignore (beg end) (beg end))

Fairly simple logic to use this.

As for tool bounds that are haphazardly split by the user, I recommend interning them into re-education camps until they stop expecting to pour skittles into the gas tank without something breaking. I'll see what's in the manual about that.

Claude Sonnet 3.0 wrote that 🙊

[karthink]

I want this for gptel's chat feature, but most other uses of tools don't have this goal.

They will. The front-ends are just rudimentary right now. The killer pattern will be having a usually hidden but editable session buffer to store and remix context. When re-writing regions, you want gptel to have seen how other source code works and to remember it for the duration of potentially 2-3 re-write attempts with different prompts. This session buffer can be used across multiple buffers, such as with occur workflows. It can be cloned to start multiple requests in parallel, forked, supports undo barriers and so on. Chat combines the idea of a working buffer and a target buffer.

Interesting idea -- but what you're describing will essentially reuse the mapping that you're creating in this PR. No one else is going to write this feature from scratch. A big chunk of tool use is going to be for side-effects, and another chunk is going to be one-off queries or queries where retaining the history is not important.

But it's also unnecessary, since INFO is a bucket meant for exactly this kind of dynamic information and variadic use.

I'm hesitant because it's a shared mutable object that lives for the entire request lifecycle. Doesn't feel right to throw such a clear function signature relationship in there.

INFO changes all the time anyway. You can do something like

(plist-put info :type 'tool-result)
(funcall (plist-get info :callback) result)
(plist-put info :type nil)

I like your later suggestion of 'tool-call in the car better.

That said, including the type in the car is fine, yeah.

So a dispatch system where RESPONSE is set to different lists is looking more appealing

Not the best, but simple enough. Recommend putting a TODO(2.0) on this?

Yeah. Scales well enough to new tasks too. The way I think about it is that every case looks like this:

(response    . "response text")
(reasoning   . "reasoning text")
(tool-call   . ((tool args closure) ...))
(tool-result . ((tool args result) ...))

only the first case is 90%+ of uses so we make an exception and directly send "response text".

Recommend:

(tool-call ((tool args closure id))...) and (tool-result ((tool args result id))...). Backends without ids can generate it.

I hope the missed dot is a typo: we want (tool-call . ((tool args closure id) ...)) and not (tool-call ((tool args closure id) ...)).

I maintain that including the id here is premature, as almost no one will need it except for us in this PR. (If an API consumer wants a persistent history of tool-calls, they'll just be reusing our UI.)

How about picking it up from INFO instead? From within the callback,

(plist-get info :tool-use)

contains all the ids, as well as other details you might need.

Tool calls appear in the last two messages in the messages array, unlikely to make much difference.

The opposite is true. As I've demonstrated, tool calls are going to wind up heavily interwoven in recursive lookup use cases. We're pretty good at sending an append-only log in many cases. It's good to be consistent.

Not quite -- the cache miss will only be for a fraction of the buffer each time, you can run through the experiment. But we might as well store the original id, yeah.

On that note, tool calls and result properties can also be shortened when serialized, to tc and tr?

We can even combine tool calls and results. It might make other problems simpler. I'll take a look.

A 2D structure is most efficient here. For enough flexibility in the values:

(response (beg end) (beg end)
 tool (beg end id) (beg end id)
 ignore (beg end) (beg end))

Fairly simple logic to use this.

No strong opinions here, except that a slight extension of the current system is about the same, since we don't need to indicate response, which is the default:

(beg end) (beg end)
(beg end tc id) (beg end tr id)
(beg end i) (beg end i)

As for tool bounds that are haphazardly split by the user, I recommend interning them into re-education camps until they stop expecting to pour skittles into the gas tank without something breaking. I'll see what's in the manual about that.

To Emacs users a buffer is a sandbox, not a gas tank. So it's perfectly reasonable from their perspective to pour skittles into it. That it represents a structured array of LLM conversation turns is unfortunately our problem to handle.

[psionic-k]

(defun gptel--modification-allow-ask (_beg _end)
  "Ask before allowing user to edit tool region."
  (if (y-or-n-p "Warning! Editing GPTel tool could break next request!
Do you know what you're doing?  Be honest, bro.")
      (save-excursion
        (when (text-property-search-backward
               'modification-hooks '(gptel--modification-allow-p) t)
          (when-let ((found (text-property-search-forward
                             'modification-hooks
                             '(gptel--modification-allow-p) t)))
            (let ((inhibit-modification-hooks t))
              (remove-text-properties (prop-match-beginning found)
                                      (prop-match-end found)
                                      '(modification-hooks
                                        insert-in-front-hooks
                                        insert-behind-hooks)))
            (message "Region modification protection removed."))))
    (user-error "Edit aborted.  You did the right thing.")))

(defun gptel--modification-undo-ask (beg end)
  "Ask user to undo edits to tool region."
  (unless (y-or-n-p "Warning! Inserting into GPTel tool could break next request!
Do you know what you're doing?  Be honest, bro.")
    (let ((inhibit-modification-hooks t))
      (delete-region beg end)
      (message "Insertion deleted"))))

(defun gptel--modification-protect (beg end)
  (add-text-properties
   beg end
   '(modification-hooks
     (gptel--modification-allow-ask)
     insert-in-front-hooks
     (gptel--modification-undo-ask)
     insert-behind-hooks
     (gptel--modification-undo-ask))))

This is what I spiked out to prevent fat fingering while allowing editing if the user insists. It is more complex than setting `'read-only'.

Instead just adding `'read-only' to a region is pretty nearly ideal. It allows killing. It's just a bit tricky to edit unless the user is industrious enough to make some Elisp.