Realtime evaluation result table update #248

andresgutgon · 2024-09-20T15:26:19Z

What?

When a evaluation is run show results in the frontend whenever they're processed

TODO

Fix weird jumping of table update when mutating with row from websockets
On finish run evaluation trigger update stats
Add test to check websocket is called when evaluation result is created

andresgutgon · 2024-09-20T15:27:07Z

packages/jobs/src/utils/progressTracker.ts

-  async decrementEnqueued() {
-    await this.redis.decr(this.getKey('enqueued'))
-  }
-


I believe these 2 were not used

packages/core/src/events/handlers/createEvaluationResultJob.test.ts

packages/core/src/services/evaluationResults/aggregations/countersQuery.test.ts

geclos · 2024-09-25T07:09:51Z

...]/documents/[documentUuid]/evaluations/[evaluationId]/_components/Content/useRefetchStats.ts

+    documentUuid,
+    evaluationId,
+  })
+  const { refetch: refetchTotals } = useEvaluationResultsCounters({


is this refetch actually the mutate function? if so i wouldn't rename it it's better to expose the real name so devs understand what they can do with this

I knew this will come out. I don't agree in this case to be honest. I think fetch express better the intention here

If this function just triggers a new request to update the data, I agree with @andresgutgon here ☝️

but it doesn't just do that, you can also pass it data to mutate the store

In general yes, but in this use case is a refetch. We're talking semantics here

geclos · 2024-09-25T07:10:48Z

...d]/evaluations/[evaluationId]/_components/EvaluationResults/EvaluationStatusBanner/index.tsx

            <ProgressIndicator state='running'>
              {`Running batch evaluation ${job.completed}/${job.total}`}
            </ProgressIndicator>
          )}
-          {job.errors > 0 && !isDone(job) && (
+          {job.errors > 0 && !isEvaluationRunDone(job) && (


actually better to not remove this message even when it's done

How they can remove the message?

navigating away

geclos · 2024-09-25T07:12:25Z

...jectId]/versions/[commitUuid]/documents/[documentUuid]/evaluations/[evaluationId]/layout.tsx

+  documentUuid: string
+  commit: Commit
+}): Promise<ReturnType<T>> {
+  const aggregationTotals = await getEvaluationTotalsQuery({


are these methods cached? add the suffix if so

Not cached. Should they be cached? It's only called once here in the server

i always cache by default tbh, might not be necessary

My rule of thumb is import { cache } from 'react' is like a Rails per request singleton. Like current_user. Only useful when is used more than once during that request.

geclos · 2024-09-25T07:13:08Z

...jectId]/versions/[commitUuid]/documents/[documentUuid]/evaluations/[evaluationId]/layout.tsx

        evaluationResults={evaluationResults}
-        evaluation={evaluation}
+        isNumeric={isNumeric}


you are already passing the evalution to the content component you don't need this derived state prop also

I use to type the component with a generic

geclos · 2024-09-25T07:14:26Z

apps/web/src/stores/evaluationResultCharts/evaluationResultsCounters.ts

 ) {
  const { project } = useCurrentProject()
+  const fetcher = useCallback(async () => {
+    const [data, error] = await computeEvaluationResultsCountersAction({


it's a pita so don't do it, but next time remember not to use server actions for fetching instead use regular api endpoints

Why? I feel like having to use regular api endpoints are more pita than server actions.

server actions are all post requests which doesn't allow for http caching, even react docs recommend not using server actions for fetching https://react.dev/reference/rsc/use-server#caveats

We've already implemented the first api endpoint so adding more it's trivial now

I agree with pain in my heart

geclos · 2024-09-25T07:18:25Z

packages/jobs/src/job-definitions/batchEvaluations/runEvaluationJob.test.ts

+      },
+    })
+  })
+})


geclos · 2024-09-25T07:18:31Z

packages/jobs/src/job-definitions/batchEvaluations/runDocumentJob.test.ts

+        evaluationId: evaluation.id,
+        documentUuid: document.documentUuid,
+      },
+    })
  })
 })


geclos · 2024-09-25T07:19:11Z

packages/core/src/events/handlers/createEvaluationResultJob.ts

+      evaluationId: evaluation.id,
+      evaluationResultId: evaluationResult.id,
+      row: evaluationResultWithMetadata,
+    },


you are including workspaceId two times? not a big deal but weird

The convention I have is that what is inside data is what is send to the browser. the workspaceId from outside is for the websocket server to know to what room has to send the info.
https://github.com/latitude-dev/latitude-llm/blob/main/apps/websockets/src/server.ts#L143

geclos · 2024-09-25T07:19:27Z

packages/core/src/events/handlers/createEvaluationResultJob.ts

+  )
+  const result = await baseQuery
+    .where(eq(evaluationResultsScope.id, evaluationResult.id))
+    .limit(1)


hmm weird way of building the query why didn't u use a repository?

baseQuery is using repositories

but why not EvaluationResultsRepository(...).find(evaluationResult.id) y ya esta?

The baseQuery brings same aggregations used in the table

packages/jobs/src/job-definitions/batchEvaluations/runEvaluationJob.ts

processed

andresgutgon commented Sep 20, 2024

View reviewed changes

andresgutgon added the 🚧 wip Work in progress label Sep 20, 2024

andresgutgon marked this pull request as ready for review September 20, 2024 15:54

andresgutgon force-pushed the feature/realtime-evaluation-result-table-update branch from 237a175 to 2c9350e Compare September 23, 2024 13:55

andresgutgon removed the 🚧 wip Work in progress label Sep 23, 2024

andresgutgon commented Sep 23, 2024

View reviewed changes

packages/core/src/events/handlers/createEvaluationResultJob.test.ts Outdated Show resolved Hide resolved

andresgutgon force-pushed the feature/realtime-evaluation-result-table-update branch 5 times, most recently from 050463f to b41280e Compare September 24, 2024 10:07

andresgutgon requested review from csansoon and geclos September 24, 2024 10:18

geclos reviewed Sep 24, 2024

View reviewed changes

packages/core/src/services/evaluationResults/aggregations/countersQuery.test.ts Show resolved Hide resolved

andresgutgon force-pushed the feature/realtime-evaluation-result-table-update branch from b41280e to 6b6097c Compare September 25, 2024 07:09

geclos reviewed Sep 25, 2024

View reviewed changes

When a evaluation is run show results in the frontend whenever they're

d5e5428

processed

andresgutgon force-pushed the feature/realtime-evaluation-result-table-update branch from 9ce87a4 to d5e5428 Compare September 25, 2024 09:55

andresgutgon merged commit 8f944ab into main Sep 25, 2024
4 checks passed

andresgutgon deleted the feature/realtime-evaluation-result-table-update branch September 25, 2024 10:24

github-actions bot locked and limited conversation to collaborators Sep 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Realtime evaluation result table update #248

Realtime evaluation result table update #248

andresgutgon commented Sep 20, 2024 •

edited

Loading

andresgutgon Sep 20, 2024

geclos Sep 25, 2024

andresgutgon Sep 25, 2024

csansoon Sep 25, 2024

geclos Sep 25, 2024

andresgutgon Sep 25, 2024

geclos Sep 25, 2024

andresgutgon Sep 25, 2024

geclos Sep 25, 2024

geclos Sep 25, 2024

andresgutgon Sep 25, 2024

geclos Sep 25, 2024

andresgutgon Sep 25, 2024

geclos Sep 25, 2024

andresgutgon Sep 25, 2024

geclos Sep 25, 2024

csansoon Sep 25, 2024

geclos Sep 25, 2024 •

edited

Loading

andresgutgon Sep 25, 2024

geclos Sep 25, 2024

geclos Sep 25, 2024

geclos Sep 25, 2024

andresgutgon Sep 25, 2024

geclos Sep 25, 2024

andresgutgon Sep 25, 2024

geclos Sep 25, 2024

andresgutgon Sep 25, 2024

Realtime evaluation result table update #248

Realtime evaluation result table update #248

Conversation

andresgutgon commented Sep 20, 2024 • edited Loading

What?

TODO

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 25, 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

andresgutgon commented Sep 20, 2024 •

edited

Loading

geclos Sep 25, 2024 •

edited

Loading