feature: Evaluations playground #2 #132

geclos · 2024-09-03T15:06:19Z

Implements the import logs modal

andresgutgon · 2024-09-04T12:33:57Z

apps/web/src/actions/documents/create.ts

    }),
    { type: 'json' },
  )
  .handler(async ({ input, ctx }) => {
    const commitsScope = new CommitsRepository(ctx.project.workspaceId)
    const commit = await commitsScope
-      .getCommitById(input.commitId)
+      .getCommitByUuid({ uuid: input.commitUuid, project: ctx.project })


andresgutgon · 2024-09-04T12:34:30Z

apps/web/src/actions/documents/destroyDocumentAction/index.test.ts

@@ -81,7 +81,7 @@ describe('destroyDocumentAction', async () => {
    it('fails when trying to remove a document from a merged commit', async () => {
      const [_, error] = await destroyDocumentAction({
        projectId: project.id,
-        commitId: merged.id,
+        commitUuid: merged.uuid,


Why this change?

we are working with commit uuids in the frontend constantly, using it in actions saves us one roundtrip to get the commit id

geclos · 2024-09-05T06:50:32Z

apps/gateway/src/routes/api/v1/chats/handlers/addMessage.ts

@@ -28,13 +28,12 @@ export const addMessageHandler = factory.createHandlers(
        workspace,
        documentLogUuid,
        messages,
-        providerLogHandler: (log) => {
-          // TODO: Review why this is possibly undefined now
-          queues.defaultQueue.jobs.enqueueCreateProviderLogJob!({


this was wrong provider logs have to be saved in sync

I understand but now the request takes longer. This is the tradeoff no?

geclos · 2024-09-05T06:50:40Z

.../gateway/src/routes/api/v1/projects/:projectId/commits/:commitUuid/documents/handlers/run.ts

@@ -42,16 +43,17 @@ export const runHandler = factory.createHandlers(
        commit,
        parameters,
        providerLogHandler: (log) => {
-          queues.defaultQueue.jobs.enqueueCreateProviderLogJob({


geclos · 2024-09-05T06:50:48Z

.../gateway/src/routes/api/v1/projects/:projectId/commits/:commitUuid/documents/handlers/run.ts

        },
      }).then((r) => r.unwrap())

      await pipeToStream(stream, result.stream)

+      // TODO: review if this is needed and why it's not in addMessages handler?


@andresgutgon question for you ☝🏼

Need what? Sorry I don't understand

queues.defaultQueue.jobs.enqueueCreateDocumentLogJob

this is being called here but it's not being called in the addMessages handler which also calls ai, I'm wondering if it should be removed from here or added to addMessages

Yes because a document log is created when a document is run. But when a message is created only the logs for the AI are created (provider log)

aaahm ok 👍🏼

Remove this comment in the next PR

geclos · 2024-09-05T06:51:47Z

apps/web/src/actions/providerLogs/fetch.ts

+    }
+
+    return result
+  })


i've implemented a regular REST get operation with filters instead of custom action for each type of get, this means that getProviderLogsForDocumentLogAction 👇🏼 is no longer needed

geclos · 2024-09-05T06:52:01Z

apps/web/src/actions/sdk/addMessagesAction.ts

@@ -28,7 +28,7 @@ export async function addMessagesAction({
  const sdk = await createSdk().then((r) => r.unwrap())
  const stream = createStreamableValue<ChainEvent, Error>()

-  const response = sdk.addMessges({


ultra typo please be careful this is public api

Sorry about that.

geclos · 2024-09-05T06:53:26Z

...evaluations/[evaluationUuid]/editor/_components/EvaluationEditor/Editor/Playground/index.tsx

+      </div>
+    </>
+  )
+}


very similar to document editor playground but not exactly the same. Probably some opportunities for DRYing but will evaluate later

geclos · 2024-09-05T06:53:57Z

.../(private)/evaluations/[evaluationUuid]/editor/_components/EvaluationEditor/Editor/index.tsx

@@ -61,7 +62,7 @@ export default function EvaluationEditor({
  useEffect(() => {
    readMetadata({
      prompt: value,
-      withParameters: Object.keys(PARAMETERS_FROM_LOG),
+      // withParameters: ['messages', 'response'], // TODO: uncomment this line once I understand why it breaks


using withParamaters fails with a "variable not declared", have to debug the compiler. For now just commenting out since everything works without it.

cc @csansoon you probably can provide some light here

Unlike with documents, evaluations do not have the ability to create new variables just by referencing them in the code. Instead, it has a bunch of predefined variables, and users can only use those. This means that, to "readMetadata", the compiler must know which are these variables names so that it knows if it would fail when you reference a parameters. If it exists in that list nothing happens, but if it doesn't, it should fail with a "variable not declared".

yes that's what i expected but the compiler fails with a "messages variable not declared" in this document, why?

The variable is declared, just not assigned a value yet

well, if withParameters contains messages, that's a bug in the compiler then 😅

talked offline, will fix it in the next pr

geclos · 2024-09-05T06:56:41Z

packages/core/src/lib/Transaction.ts

@@ -50,6 +51,10 @@ export default class Transaction {
   * https://github.com/rails/rails/blob/main/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb#L769.
   */
  static toResultError(error: unknown): ErrorResult<Error> {
+    if (env.NODE_ENV === 'development') {
+      console.error(error)
+    }


make our lives easier

geclos · 2024-09-05T06:57:30Z

packages/core/src/repositories/providerLogsRepository.ts

+    const result = await query
+
+    return Result.ok(result)
+  }


pagination very needed here since we expect 10s of thousands of logs

We have this https://github.com/latitude-dev/latitude-llm/blob/main/packages/core/src/repositories/repository.ts#L22-L30

hmm seems like very low level no? I don't want to reimplement the findAll query, just add some pagination to it

What do you mean? Is the official way of doing pagination in drizzle

no not talking about drizzle's api saying that if i already have the find and findall methods implementing in repositories i don't want to call another method to have the same but paginated, i just wanna indicate pagination as an options of those methods.

I will use page and pageSize instead of limit and offset though, I think it makes more sense, will do in the next PR

geclos · 2024-09-05T06:58:43Z

packages/core/src/repositories/repository.ts

+    }
+
+    if (opts.offset !== undefined) {
+      // @ts-expect-error


since this is an abstract class implementing operations over a query that is defined by the children classes typescript has difficulties understanding the types, adding a bit of technical debt here in favour of moving forward towards the MVP

Check if this comment can help https://github.com/latitude-dev/latitude-llm/pull/132/files#r1744961217

geclos · 2024-09-05T07:00:21Z

packages/core/src/services/ai/estimateCost/index.ts

-  const inputCost = (inputCostPer1MToken * (inputTokens ?? 0)) / 1_000_000
-  const outputCost = (outputCostPer1MToken * (outputTokens ?? 0)) / 1_000_000
+  const inputCost =
+    (inputCostPer1MToken * (isNaN(inputTokens) ? 0 : inputTokens)) / 1_000_000


this was a bug, vercel's ai can return NaN in some token usage for some providers. Probably a bug on their end but regardless we have to defend ourselves.

geclos · 2024-09-05T07:01:07Z

packages/web-ui/src/ds/atoms/Modal/index.tsx

 }: ModalProps) {
  return (
    <Dialog open={open} defaultOpen={defaultOpen} onOpenChange={onOpenChange}>
-      <DialogContent className='sm:max-w-modal'>


this class wasn't getting applied because DialogContent was doing nothing with the prop className

csansoon · 2024-09-05T07:07:57Z

...evaluations/[evaluationUuid]/editor/_components/EvaluationEditor/Editor/Playground/index.tsx

+  useEffect(() => {
+    if (!metadata) return
+
+    // Remove only inputs that are no longer in the metadata, and add new ones
+    // Leave existing inputs as they are
+    setInputs(
+      Object.fromEntries(
+        Array.from(metadata.parameters).map((param) => {
+          if (param in inputs) return [param, inputs[param]!]
+
+          return [param, { value: '', type: 'input' }]
+        }),
+      ),
+    )
+  }, [metadata])


There cannot be "new inputs". Evaluation only have the default predefined parameters that are extracted from a log.

hmm but the user can edit the evaluation prompt and remove the predefined parameters and add them back in, no? Nothing prevents me from editing the text on the left here

hmm weird ux imo, I'd rather show them a message telling them they have to include the messages variable if it's not present in the document, rather than showing an input for a variable that they have removed from the document

Yeah, but the parameters always exist, wether you use them in the prompt or not. It is not like Documents, where ONLY when you reference a variable in the code a new parameter is created. In this case, users must only reference variables from a list of predefined variables, which are filled with data automatically from logs. I think we should always display this list to the user, so they’re aware of the parameters they have access to, even if they’re not currently using them in their code.

(resending this message, this was removed by accident)

but if they don't use it in the document they won't have an effect at all when you run the prompt no? it's just weird to have an input for a variable that won't have any usage in the document. I understand that internally they exist in the state of the compiler when running the document, but that's not relevant to the user. It's better to tell the user "hey, remember to add the messages variable to your document" rather than just showing an input that has no effect on the evaluation

Maybe we shouldn't display it as a bunch of inputs. We could just display a "log" there, with the ability to modify it.

...evaluations/[evaluationUuid]/editor/_components/EvaluationEditor/Editor/Playground/index.tsx

andresgutgon · 2024-09-05T07:32:42Z

apps/web/src/app/(private)/evaluations/[evaluationUuid]/editor/import-logs/page.tsx

+    default:
+      return 'default'
+  }
+}


Classic hash cc @ferranete

Modal to import logs while testing the evaluation in the editor

geclos force-pushed the feature/evaluations_playground branch from 4b7f345 to db027e1 Compare September 4, 2024 12:06

andresgutgon reviewed Sep 4, 2024

View reviewed changes

geclos force-pushed the feature/evaluations_playground branch from db027e1 to ad7ebac Compare September 5, 2024 06:49

geclos commented Sep 5, 2024

View reviewed changes

geclos force-pushed the feature/evaluations_playground branch from ad7ebac to dea4ce5 Compare September 5, 2024 06:56

geclos commented Sep 5, 2024

View reviewed changes

geclos force-pushed the feature/evaluations_playground branch from dea4ce5 to 6bd65c2 Compare September 5, 2024 07:01

geclos marked this pull request as ready for review September 5, 2024 07:01

geclos force-pushed the feature/evaluations_playground branch 2 times, most recently from 1c7dda6 to bff38e0 Compare September 5, 2024 07:05

csansoon reviewed Sep 5, 2024

View reviewed changes

geclos force-pushed the feature/evaluations_playground branch 2 times, most recently from f9bf84c to 34f1c7e Compare September 5, 2024 07:09

csansoon reviewed Sep 5, 2024

View reviewed changes

...evaluations/[evaluationUuid]/editor/_components/EvaluationEditor/Editor/Playground/index.tsx Outdated Show resolved Hide resolved

geclos force-pushed the feature/evaluations_playground branch from 34f1c7e to 48fc82c Compare September 5, 2024 07:30

andresgutgon reviewed Sep 5, 2024

View reviewed changes

feature: Import logs modal

1db6b6d

Modal to import logs while testing the evaluation in the editor

geclos force-pushed the feature/evaluations_playground branch from 48fc82c to 1db6b6d Compare September 5, 2024 07:39

geclos merged commit 6391a0d into main Sep 5, 2024
3 checks passed

geclos deleted the feature/evaluations_playground branch September 5, 2024 08:05

feature: Evaluations playground #2 #132

feature: Evaluations playground #2 #132

Conversation

geclos commented Sep 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geclos Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geclos Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geclos Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geclos commented Sep 3, 2024 •

edited

Loading

geclos Sep 5, 2024 •

edited

Loading

geclos Sep 5, 2024 •

edited

Loading

geclos Sep 5, 2024 •

edited

Loading