Fixes ENG-1430: Add eval run utility in TS

[andreibratu]

Based on #7

QA'd using the following cookbooks:

• https://github.com/humanloop/humanloop-cookbook/tree/ts-utilities-example/ts-instrumented-chat-agent
• https://github.com/humanloop/humanloop-cookbook/tree/ts-utilities-example/ts-evaluating-agent
• https://github.com/humanloop/humanloop-cookbook/tree/ts-utilities-example/deno-instrumented-chat-agent

Remarks:

• Decorators are forced to use a (inputs, messages) => ... signature. This can be bypassed by wrapping the function returned by the utility in an arrow function:

    const callLLM = async (messages: any[]) => {
        return await promptUtilityFactory(
            opentelemetryTracer,
            async (_: any, messages: any[]) => {
                const client = new OpenAI({ apiKey: process.env.OPENAI_KEY });
                const response = await client.chat.completions.create({
                    model: "gpt-4o",
                    messages: messages,
                    temperature: 0.8,
                });
                return (
                    (response.choices[0].message.content || "") +
                    " " +
                    (await randomString())
                );
            },
            "Call LLM",
        )({}, messages);
    };

    const agentCall = async (messages: any[]) =>
        await humanloop.flow(
            opentelemetryTracer,
            async (_: any, messages: any[]) => {
                return await callLLM(messages);
            },
            "Small Flow",
        )({}, messages);

• Eval run works

[harry-humanloop]

what does "will stale" mean?

[andreibratu]

Ack, made comment more detailed

[harry-humanloop]

why the ! assertion?

i think that !s should be avoided in general. if code changes such that this assumption can't be made, this wouldn't raise an error (and when it does crash, it's not that useful an error - an error message explaining why we expect state to be defined would be more helpful for a future developer coming to this and wondering why this assertion is here)

[andreibratu]

ack, added explicit check

[harry-humanloop]

"workers" here doesn't really make sense i think - does this actually spin off multiple workers? or is it just async concurrency?

does js have a standard name for this? (i do know this means inconsistency with python, but i think it's wrong enough that i'd push to change it here)

[andreibratu]

p-map limits the number of promises running at any time to be workers.

I think it makes sense with an extra modification: we should cap the number of workers to not DOS our backend. Since the promises fire up simultaneously, we might have thousands of open HTTP connections and overwhelm the backend.

Added a 32 cap.

[harry-humanloop]

my comment here was more about the specific terminology of "worker".

[harry-humanloop]

looking at p-map, it looks like they use "concurrency", which i'd prefer here as it's more accurate, even if it's inconsistent with python. (seems reasonable as it's a language-specific runtime option)

[andreibratu]

ack - I'll change to concurrency

[andreibratu]

If "consistent with python" was a goal for this PR we should have closed it long ago 😄

[harry-humanloop]

why +s and not template string? (think template string would be much cleaner)

[andreibratu]

ack changed to template

[harry-humanloop]

do we want to just check version_id? is a deep equal necessary?

[andreibratu]

version can be a deep object, e.g. {flow: attributes: {...}}

[harry-humanloop]

if both have version_id, i think we should use version_id.

as noted in other comment:
the deep equal here is slightly concerning - very often the response a user would get from the API won't be exactly equal to the request made. (e.g. the response has default values injected, along with some null attributes)

[andreibratu]

I've been considering it, and I think we should prevent the user from specifying a value for file.version when the callable is wrapped in an SDK utility. There's a mini design-doc on this issue here:

https://linear.app/humanloop/issue/ENG-1456/run-utility-callable-version-issue

I'd rather leave this as it is and revisit when we tackle the ticket above. But I'm aware it's not ideal

[andreibratu]

Discussed in a sync with Harry. Was scoped out for later

[harry-humanloop]

is our terminology here "local evaluators"?
in Humanloop we call them "External Evaluators".
is this something taken from the python sdk?

[andreibratu]

Yes - should I change to externalEvaluators?

[harry-humanloop]

yes to which question?

[harry-humanloop]

not clear to me what this should be. but it should be consistent and intuitive. neither "local" nor "external" seem great.

[andreibratu]

Yes it's taken from python SDK. I think it's good enough

[harry-humanloop]

when is this shown to users?

[harry-humanloop]

ah i see

i think this is a very passive aggressive thing to tell users.
instead of "did you forget to pass them?", we should say something like "Prompt File utility can only be used when a provider is passed in the Humanloop constructor." or "Humanloop cannot find ... . To ..., pass a provider into the HumanloopClient constructor.".

Also I think the term "Prompt File utility" is very confusing and ambiguous. We should instead specify which function was used. (Unclear why "File" important here; unclear what counts as a "utility" here - and so hard to figure out what's wrong - are all sdk methods under .prompts "prompt utilities"?;)

[andreibratu]

How about this:
${func.name}: You must pass at least one LLM client library in the Humanloop client constructor. Otherwise the prompt() utility will not work properly.

[harry-humanloop]

does passing any llm client library work? or does it have to be the one that is called within the function wrapped by prompt()?

[andreibratu]

The instrumentors require the module and modify the prototype of the client itself. So all client instances would be watched by the OTel instrumentor:
https://github.com/traceloop/openllmetry-js/blob/main/packages/instrumentation-openai/src/instrumentation.ts#L55

[harry-humanloop]

${func.name}: To use the `prompt()` utility, pass your LLM client library into the Humanloop client constructor; e.g. `HumanloopClient(..., { providerModules: {OpenAI} } )`.

clarified; should pass in all relevant llm client libraries.

[harry-humanloop]

are the 3 cookbooks meant to be released to users?
(if so, would expect a PR so I can leave comments on it.)

[andreibratu]

are the 3 cookbooks meant to be released to users? (if so, would expect a PR so I can leave comments on it.)

Yes, will make them public after we release a beta version and can install humanloop properly from npm

[harry-humanloop]

the deep equal here is slightly concerning - very often the response a user would get from the API won't be exactly equal to the request made. (e.g. the response has default values injected, along with some null attributes)

is that relevant here?

Are both state.evaluatedVersion and request.flow user-provided?
Should we check explicitly on version_id instead (if that is available)

[andreibratu]

Discussed in a sync with Harry. Was scoped out for later