Deprecate getLogs method from Ai binding

[G4brym]

This was a testing method added to debug the Ai binding, and is now being deprecated.
It no longer returns any data from the server

[Cherry]

Is this safe to remove without potentially causing userland runtime issues? Although not officially documented, it's been listed, available, and accessible in the types via @cloudflare/ai for a while:

[G4brym]

From my point of view, end users don't get any benefit from what's being returned here
And this method is going to stop returning results from server side soon, so the client method will have nothing to show

[Cherry]

Agreed, but returning an empty array likely won't break applications, whereas a method being removed that was previously there will cause a TypeError and potentially break userland code if called and expected to be present.

[G4brym]

Also the new Ai binding is like 2 weeks old, so provably very few users have seen this method, and if they are calling it from the old @cloudflare/ai npm package, they can continue calling it, its just not going to show anything, but it will work

[Cherry]

so provably very few users have seen this method

Are there any stats for this? This is a breaking change and should probably be behind a compat flag imo. Workers AI is GA.

[CyebSloth]

Hey @G4brym ,

what Information are you looking to get from the getLogs

The result before any postProcessing is executed, the raw output can be used as input for other models or could be a requirement when the customer wants to apply a different post-processing technique.

On an ideal scenario, we'd be able to "hook" intermediate activations and gradients, potentially allowing customers to use existing models for more advanced purposes (feature extraction with resnet and interpretation with llama + loras, for example).

Thank you for the follow-up!

[G4brym]

I will forward your feature suggestion internally 👍

[G4brym]

In another note, I've come to realize this getLogs method actually never worked when using the new binding, because the run method was throwing an error when parsing the logs from the server.
So for anyone to be using this method it's guaranteed that you are using the old npm package.
So in reality this is not a breaking change, because it never worked on the new binding.

With this in mind, in my opinion a good compromise resolution for this is to:
Merge this PR that removes this method as it never returned anything besides an empty array, and keep supporting the getLogs from the npm package for the foreseeable future
Please fell free to comment, I'm open for suggestions

[Cherry]

Even if it "never worked" as intended, it still returned an empty array. Code like this works in production today, using the new binding:

export default {
	async fetch(request: Request, env: Env, ctx: ExecutionContext): Promise<Response> {
		const messages = [
			{ role: "system", content: "You are a friendly assistant" },
			{
				role: "user",
				content: "What is the origin of the phrase Hello, World",
			},
		];
		const response = await env.AI.run("@cf/meta/llama-2-7b-chat-fp16", { messages, debug: true });
		const logs = env.AI.getLogs();

		return Response.json({response, logs})
	},
};

Even with logs being an empty array, it's not breaking anything in my code, after migrating from the old @cloudflare/ai package. With the change in this PR, that code won't work anymore with no change of my own, as getLogs will now throw an error in my code, potentially breaking my application.

I would suggest if wanting to remove this, it's done so gracefully by just return [], and mark as deprecated. Or, gate the entire method removal behind a compat flag.

[G4brym]

just updated the pr to deprecate the getLogs, instead of removing it completely 👍