js[patch]: simple evaluator args

[baskaryan]

js equivalent of #1200

[jacoblee93]

I think the better/safer way to do this is to shim it with no source code inspection:

return evaluator({
         ...run,
          run,
          example,
          inputs: example?.inputs,
          outputs: run?.outputs,
          referenceOutputs: example?.outputs,,
        }, example);

Convention in JS is to ignore extra kwargs and args so it should work without breaking anyone.

Downside is it makes logging a little bit funkier but we can deprecate the old style(?) in the future

You could also maybe try this:

https://stackoverflow.com/questions/4848149/get-a-functions-arity

[jacoblee93]

CC @nfcampos I think I heard you should try to avoid Function.toString() but I'm not actually sure why (other than the fact that the regexes for some of the below checks should be tweaked)

Perf probably isn't super important given this will only run once

[jacoblee93]

Googling around and tinkering in my terminal - implementation seems unclear, I think it may have some funny edge cases (at the very least arrow functions would definitely need a separate regex, along with async, maybe others I'm not thinking about)

Would prefer to do something like the shim above in order to avoid any potential gotchas

[jacoblee93]

Googling around and tinkering in my terminal - implementation seems unclear, I think it may have some funny edge cases (at the very least arrow functions would definitely need a separate regex, along with async, maybe others I'm not thinking about)

Would prefer to do something like the shim above in order to avoid any potential gotchas

[jacoblee93]

Would also need to account for at the very least arrow-declared functions () => {} and async () => {}

[jacoblee93]

We should probably also update the types for RunEvaluatorLike above right?

[jacoblee93]

Don't need the Promise.resolve, more standard to just make the outside function async:

      const wrapperSuperInner = traceable(
        async (
          _runs_: string,
          _examples_: string
        ): Promise<EvaluationResult | EvaluationResults> => {

[jacoblee93]

Should we just do the arity check for both of these?

Is there a world where there are some crazies out there declaring evaluators like this:

async (run) => {
}

I would prefer the shim in both places unless we want to drop support for the two arg syntax in the long run, in which case we should deprecate the other signature:

export type RunEvaluatorLike =
 /** @deprecated NOTE */
  | ((
      run: Run,
      example?: Example
    ) => Promise<EvaluationResult | EvaluationResults>)
 /** @deprecated NOTE */
  | ((run: Run, example?: Example) => EvaluationResult | EvaluationResults)
  | ((args: {
      run?: Run;
      example?: Example;
      inputs?: Record<string, any>;
      outputs?: Record<string, any>;
      referenceOutputs?: Record<string, any>;
    }) => EvaluationResult | EvaluationResults)
  | ((args: {
      run?: Run;
      example?: Example;
      inputs?: Record<string, any>;
      outputs?: Record<string, any>;
      referenceOutputs?: Record<string, any>;
    }) => Promise<EvaluationResult | EvaluationResults>);

[dqbd]

Worth supporting evaluator(runs)?