Evaluation dashboard #146

csansoon · 2024-09-09T10:34:14Z

csansoon · 2024-09-09T13:42:04Z

...ations/(evaluation)/[evaluationUuid]/dashboard/_components/ConnectedDocumentsTable/index.tsx

+            onSelect={() =>
+              navigate.push(
+                ROUTES.projects
+                  .detail({ id: document.projectId })
+                  .commits.detail({ uuid: HEAD_COMMIT })
+                  .documents.detail({ uuid: document.documentUuid }).logs.root, // TODO: Navigate to the document evaluation details page
+              )
+            }


Navigating to document logs as a mock rn. It will redirect to the document evaluation details page when it's available.

it's available! check routes in main

csansoon · 2024-09-09T14:23:05Z

packages/core/src/repositories/connectedEvaluationsRepository.ts

+  async getConnectedDocumentsWithMetadata(
+    evaluationId: number,
+  ): Promise<TypedResult<ConnectedDocumentWithMetadata[], LatitudeError>> {


Attention: black magic ahead. Complex sql query to fetch aggregated data from connected evaluations. The idea being: Display all documents connected to this evaluation, and the costs and metadata of the results. The problem is that the relations from connectedEvaluations to this info is not really straight-forward.

geclos · 2024-09-10T08:37:15Z

...ations/(evaluation)/[evaluationUuid]/dashboard/_components/ConnectedDocumentsTable/index.tsx

+        <Text.H4 color='foregroundMuted' noWrap>
+          {promptPath}
+          {Boolean(promptPath) && '/'}
+          <Text.H4 noWrap>{promptName}</Text.H4>


I would truncate this name after a certain number of characters, the preview you shared doesn't look very good with very long path names 😅

geclos · 2024-09-10T08:37:22Z

...ations/(evaluation)/[evaluationUuid]/dashboard/_components/ConnectedDocumentsTable/index.tsx

+      </TableCell>
+      <TableCell>
+        {isProjectsLoading ? (
+          <Skeleton className='w-full h-4 bg-muted animate-pulse' />


nice 👌🏼

...private)/evaluations/(evaluation)/[evaluationUuid]/dashboard/_components/EvaluationStats.tsx

geclos · 2024-09-10T08:44:33Z

packages/core/src/repositories/connectedEvaluationsRepository.ts

+      .from(documentVersionsScope.scope)
+      .where(isNotNull(documentVersionsScope.scope.mergedAt)) // Only account for merged documents
+      .groupBy(documentVersionsScope.scope.documentUuid)
+      .as('latest_document_versions')


hmm this should be in the document versions repo but i understand why it ain't straightforward to put it there... anyhow let's continue and we'll think on how to improve our repos later on

geclos · 2024-09-10T08:45:41Z

packages/core/src/repositories/connectedEvaluationsRepository.ts

+      .where(eq(this.scope.evaluationId, evaluationId))
+
+    return Result.ok(result.filter((r) => r.deletedAt == null)) // Only show non-removed documents
+  }


this merits a test case tbh

Venga que cursor te los escribe solo casi

☝️ Rewrote the queries to add a modal value, and simplify the overall query complexity. Working on tests now!

Done! Had to modify some factories and a few other functions, but the test env is a bit cleaner now!

andresgutgon · 2024-09-10T14:57:31Z

packages/core/src/services/evaluations/connect.ts

+): PromisedResult<ConnectedEvaluation[], Error> {
+  return Transaction.call(
+    async (tx): PromisedResult<ConnectedEvaluation[], Error> => {
+      const documentVersionsScope = new DocumentVersionsRepository(


No need to put this inside a transaction. Keep it outside.

I need to use a transaction, because it creates new evaluations from the templates before creating the connections! If anything fails in between those two actions, I'd like to undo the changes.

andresgutgon · 2024-09-10T14:59:21Z

packages/core/src/services/evaluations/create.ts

@@ -43,3 +44,29 @@ export async function createEvaluation(
    return Result.ok({ ...result[0]!, metadata: metadataTable[0]! })
  }, db)
 }
+
+export async function importLlmAsJudgeEvaluation(


Put this method where it is used. No need to put in create service

andresgutgon · 2024-09-10T15:02:34Z

packages/core/src/repositories/documentVersionsRepository/index.ts

+    ) {
+      // Note: otherwise the comparison fails with "invalid input syntax for type uuid: 'non-existent-uuid'""
+      return false
+    }


Don't do this. Wrap in a transaction and it will return a result error. This is a legit SQL error

But I don't want to return false for any error! I only want to return false if there's not any document in the scope with the given uuid. If there is an error from any other reason, I still want the error to be thrown.

andresgutgon · 2024-09-10T15:03:01Z

packages/core/src/repositories/connectedEvaluationsRepository/index.ts

+        eq(selectedDocuments.documentUuid, modalValueCount.documentUuid),
+      )
+
+    return Result.ok(result.filter((r) => r.deletedAt == null)) // Only show non-removed documents


Thanks! 😂

geclos · 2024-09-10T21:38:56Z

apps/web/src/actions/evaluations/connect.ts

+      workspace: ctx.workspace,
+      documentUuid: input.documentUuid,
+      evaluationUuids: input.evaluationUuids,
+      templateIds: input.templateIds,


Services should always receive model instances, not identifiers, and tenancy checks should live at the boundaries of the application (actions, in this case) 🙏🏼 This is mainly to ensure developers always implement tenancy when adding public endpoints.

I know you are passing the workspace and probably implementing tenancy through that internally but it breaks the pattern and sets us up for the next dev to implement a service without a proper tenancy check.

geclos · 2024-09-10T21:40:32Z

packages/core/src/services/evaluations/connect.ts

+        await documentVersionsScope.existsDocumentWithUuid(documentUuid)
+      if (!documentExists) {
+        return Result.error(new NotFoundError('Document not found'))
+      }


not this service responsibility

This service had tests that checked that it failed when the document didn't exist

Well, not. Actually the tests were about the server action, not the service...

geclos · 2024-09-10T21:42:09Z

packages/core/src/services/evaluations/connect.ts

+            `The following evaluations were not found: ${missingEvaluationUuids?.join(', ')}`,
+          ),
+        )
+      }


coolio but it belongs to the action not this service

geclos

awesome work 👌🏼 my only gripe is with tenancy checks

csansoon added the 🚧 wip Work in progress label Sep 9, 2024

csansoon force-pushed the evaluation-dashboard branch 5 times, most recently from 0ba2fe4 to 4125fed Compare September 9, 2024 13:40

csansoon commented Sep 9, 2024

View reviewed changes

csansoon force-pushed the evaluation-dashboard branch 2 times, most recently from fa19382 to 6dd0730 Compare September 9, 2024 14:19

csansoon commented Sep 9, 2024

View reviewed changes

csansoon force-pushed the evaluation-dashboard branch 4 times, most recently from 1b39558 to 86fd8e0 Compare September 9, 2024 16:02

csansoon marked this pull request as ready for review September 9, 2024 16:12

csansoon removed the 🚧 wip Work in progress label Sep 9, 2024

csansoon force-pushed the evaluation-dashboard branch from 86fd8e0 to 91a2107 Compare September 10, 2024 07:39

geclos reviewed Sep 10, 2024

View reviewed changes

...private)/evaluations/(evaluation)/[evaluationUuid]/dashboard/_components/EvaluationStats.tsx Show resolved Hide resolved

geclos reviewed Sep 10, 2024

View reviewed changes

csansoon force-pushed the evaluation-dashboard branch 6 times, most recently from f485efe to d4bfb94 Compare September 10, 2024 14:44

andresgutgon reviewed Sep 10, 2024

View reviewed changes

andresgutgon previously approved these changes Sep 10, 2024

View reviewed changes

Evaluation dashboard

345311b

csansoon dismissed andresgutgon’s stale review via 345311b September 10, 2024 15:19

csansoon force-pushed the evaluation-dashboard branch from d4bfb94 to 345311b Compare September 10, 2024 15:19

geclos reviewed Sep 10, 2024

View reviewed changes

geclos approved these changes Sep 10, 2024

View reviewed changes

csansoon merged commit 7050747 into main Sep 11, 2024
3 checks passed

csansoon deleted the evaluation-dashboard branch September 11, 2024 07:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Evaluation dashboard #146

Evaluation dashboard #146

csansoon commented Sep 9, 2024 •

edited

Loading

csansoon Sep 9, 2024

geclos Sep 10, 2024 •

edited

Loading

csansoon Sep 9, 2024

geclos Sep 10, 2024

geclos Sep 10, 2024

geclos Sep 10, 2024 •

edited

Loading

geclos Sep 10, 2024

csansoon Sep 10, 2024

geclos Sep 10, 2024

csansoon Sep 10, 2024

csansoon Sep 10, 2024

andresgutgon Sep 10, 2024

csansoon Sep 10, 2024

andresgutgon Sep 10, 2024 •

edited

Loading

andresgutgon Sep 10, 2024

csansoon Sep 10, 2024

andresgutgon Sep 10, 2024

csansoon Sep 10, 2024

geclos Sep 10, 2024 •

edited

Loading

geclos Sep 10, 2024

csansoon Sep 11, 2024

csansoon Sep 11, 2024

geclos Sep 10, 2024

geclos left a comment

Evaluation dashboard #146

Evaluation dashboard #146

Conversation

csansoon commented Sep 9, 2024 • edited Loading

Choose a reason for hiding this comment

geclos Sep 10, 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

geclos Sep 10, 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

andresgutgon Sep 10, 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

geclos Sep 10, 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

geclos left a comment

Choose a reason for hiding this comment

csansoon commented Sep 9, 2024 •

edited

Loading

geclos Sep 10, 2024 •

edited

Loading

geclos Sep 10, 2024 •

edited

Loading

andresgutgon Sep 10, 2024 •

edited

Loading

geclos Sep 10, 2024 •

edited

Loading