-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
Fixing Tool Calling #11
Conversation
@matrushka great! also trying to get ollama provider with function calling to work for https://github.com/louis030195/screen-pipe desktop app facing this with llama3.1 atm:
will test ur PR prob happy to help anyhow or jump in a call can DM on discord/x at |
Thanks @matrushka , I am traveling today and will not have time to check the PR until tomorrow. Is this using the new Ollama 0.3 API? |
Tested it with ollama/ollama:0.3.0 and also weirdly 0.2.7 :) The reference data structure I used is same in both versions (last update 2 weeks ago) https://github.com/ollama/ollama/blob/v0.3.0/server/testdata/tools/messages.json |
@matrushka any idea how i can install this branch as deps in my package.json? tried things here but didnt work https://stackoverflow.com/questions/16350673/depend-on-a-branch-or-tag-using-a-git-url-in-a-package-json prob because it's a sub dir (i can't wait tomorrow for the merge) |
Pull the repo to your local, build it |
@matrushka thx still fails: const screenpipeQuery = z.object({
q: z
.string()
.describe(
"The search query matching exact keywords. Use a single keyword that best matches the user intent"
),
contentType: z
.enum(["ocr", "audio", "all"])
.describe(
"The type of content to search for: screenshot data or audio transcriptions"
),
limit: z
.number()
.default(5)
.describe(
"Number of results to return (default: 5). Don't return more than 50 results as it will be fed to an LLM"
),
offset: z.number().default(0).describe("Offset for pagination (default: 0)"),
startTime: z
.string()
// 1 hour ago
.default(new Date(Date.now() - 3600000).toISOString())
.describe("Start time for search range in ISO 8601 format"),
endTime: z
.string()
.default(new Date().toISOString())
.describe("End time for search range in ISO 8601 format"),
});
const screenpipeMultiQuery = z.object({
queries: z.array(screenpipeQuery),
});
async function queryScreenpipeNtimes(
params: z.infer<typeof screenpipeMultiQuery>
) {
return Promise.all(params.queries.map(queryScreenpipe));
}
// Add this new function to handle screenpipe requests
async function queryScreenpipe(params: z.infer<typeof screenpipeQuery>) {
try {
const queryParams = new URLSearchParams({
q: params.q,
offset: params.offset.toString(),
limit: params.limit.toString(),
start_date: params.startTime,
end_date: params.endTime,
content_type: params.contentType,
});
console.log("calling screenpipe", JSON.stringify(params));
const response = await fetch(`http://localhost:3030/search?${queryParams}`);
if (!response.ok) {
const text = await response.text();
throw new Error(`HTTP error! status: ${response.status} ${text}`);
}
const result = await response.json();
console.log("result", result);
return result;
} catch (error) {
console.error("Error querying screenpipe:", error);
return null;
}
}
const text = await generateText({
model: useOllama ? ollama(model) : provider(model),
tools: {
query_screenpipe: {
description:
"Query the local screenpipe instance for relevant information. You will return multiple queries under the key 'queries'.",
parameters: screenpipeMultiQuery,
execute: queryScreenpipeNtimes,
},
},
toolChoice: "required",
messages: [
// ...
|
@louis030195 seems like you couldn't configure it properly (or maybe forgot to run |
got it working!! llama3.1 function calling is on @matrushka had to do this hack though: const { textStream } = ollama
? await streamText({
model: ollama(model),
prompt: JSON.stringify([
{
role: "user",
content:
messages.findLast((msg) => msg.role === "user")?.content ||
inputMessage,
},
{
role: "assistant",
content: text.toolCalls,
},
{
role: "tool",
content: text.toolResults,
},
]),
})
: await streamText({
model: provider(model),
messages: [
{
role: "user",
content:
messages.findLast((msg) => msg.role === "user")?.content ||
inputMessage,
},
{
role: "assistant",
content: text.toolCalls,
},
{
role: "tool",
content: text.toolResults,
},
],
}); basically go json umarshall error, suspecting the RAG data was in weird format in message so just sent a string |
I had a similar issue as well, seems like when your tool returns an object instead of a string, ollama throws that error. It went away when I encoded the tool response to JSON. Might be interesting to implement that behavior so user don't need to care. |
Hi @matrushka! I think this is not working with the new API. I did a tool call inference as it was working because I was injecting the tools at the system prompt and was able to detect if the response contains a json to infer the tool response.
you can see the json does not contain the tool info but the json inside the answer If I remove the system tool prompt injection
Now content is empty and the Ollama answer contains the tool in the right property. |
@matrushka I forgot to say that I have tested with ollama3.1. It may be that some models give more priority to the system prompt, than to tool suggestion. So it is likely that depending on the model it will work one way or the other. In any case, the injection prompt is no longer necessary. Do you mind removing it, or if you prefer I accept your PR as is and I'll take care of finishing it. |
@sgomez found what you mean and did the changes. Kinda saw similar results to what you said, as depending on the model it might tend to use the |
Thanks @matrushka! Sorry but I am in holidays and I cannot check this as fast as I'd like it. There are some dead code now, but nothing to worries. I will remove it before the next release. |
Existing implementation expects function call arguments as strings (instead of objects) and the roundtrip for feeding back the result requires an updated message structure.
Also updated the @ai/provider dependencies and adjusted the tests to make everything is up-to-date.
Tested with mistral and was able to get it working well