chore(scores): drop fk constraint on traces/observations (#2014)

* chore(scores): drop fk on traces and observations * trpc update * push
langfuse · May 13, 2024 · 900a309 · 900a309
1 parent a6c033d
commit 900a309
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 92 deletions.
diff --git a/.../prisma/migrations/20240508140549_scores_drop_fk_on_traces_and_observations/migration.sql b/.../prisma/migrations/20240508140549_scores_drop_fk_on_traces_and_observations/migration.sql
@@ -0,0 +1,5 @@
+-- DropForeignKey
+ALTER TABLE "scores" DROP CONSTRAINT "scores_observation_id_fkey";
+
+-- DropForeignKey
+ALTER TABLE "scores" DROP CONSTRAINT "scores_trace_id_fkey";
diff --git a/packages/shared/prisma/schema.prisma b/packages/shared/prisma/schema.prisma
@@ -231,7 +231,6 @@ model Trace {
     createdAt  DateTime      @default(now()) @map("created_at")
     updatedAt  DateTime      @default(now()) @updatedAt @map("updated_at")
 
-    scores          Score[]
     DatasetRunItems DatasetRunItems[]
     DatasetItem     DatasetItem[]
     JobExecution    JobExecution[]
@@ -306,7 +305,6 @@ model Observation {
     outputCost          Decimal?          @map("output_cost")
     totalCost           Decimal?          @map("total_cost")
     completionStartTime DateTime?         @map("completion_start_time")
-    scores              Score[]
     project             Project           @relation(fields: [projectId], references: [id], onDelete: Cascade)
     derivedDatasetItems DatasetItem[]
     datasetRunItems     DatasetRunItems[]
@@ -400,9 +398,7 @@ model Score {
     source        ScoreSource
     comment       String?
     traceId       String         @map("trace_id")
-    trace         Trace          @relation(fields: [traceId], references: [id], onDelete: Cascade)
     observationId String?        @map("observation_id")
-    observation   Observation?   @relation(fields: [observationId], references: [id], onDelete: SetNull)
     JobExecution  JobExecution[]
 
     @@unique([id, traceId]) // used for upsert

diff --git a/web/src/server/api/routers/scores.ts b/web/src/server/api/routers/scores.ts
@@ -2,7 +2,6 @@ import { z } from "zod";
 
 import {
   createTRPCRouter,
-  protectedProcedure,
   protectedProjectProcedure,
 } from "@/src/server/api/trpc";
 import { throwIfNoAccess } from "@/src/features/rbac/utils/checkAccess";
@@ -101,9 +100,7 @@ export const scoresRouter = createTRPCRouter({
     .query(async ({ input, ctx }) => {
       const names = await ctx.prisma.score.groupBy({
         where: {
-          trace: {
-            projectId: input.projectId,
-          },
+          projectId: input.projectId,
         },
         by: ["name"],
         _count: {
@@ -117,29 +114,10 @@ export const scoresRouter = createTRPCRouter({
 
       return res;
     }),
-  byId: protectedProcedure.input(z.string()).query(({ input, ctx }) =>
-    ctx.prisma.score.findFirstOrThrow({
-      where: {
-        id: input,
-        ...(ctx.session.user.admin === true
-          ? undefined
-          : {
-              trace: {
-                project: {
-                  projectMembers: {
-                    some: {
-                      userId: ctx.session.user.id,
-                    },
-                  },
-                },
-              },
-            }),
-      },
-    }),
-  ),
-  createReviewScore: protectedProcedure
+  createReviewScore: protectedProjectProcedure
     .input(
       z.object({
+        projectId: z.string(),
         traceId: z.string(),
         value: z.number(),
         name: z.string(),
@@ -162,23 +140,33 @@ export const scoresRouter = createTRPCRouter({
       });
       throwIfNoAccess({
         session: ctx.session,
-        projectId: trace.projectId,
+        projectId: input.projectId,
         scope: "scores:CUD",
       });
 
+      const trace = await ctx.prisma.trace.findFirst({
+        where: {
+          id: input.traceId,
+          projectId: input.projectId,
+        },
+      });
+      if (!trace) {
+        throw new Error("No trace with this id in this project.");
+      }
+
       const score = await ctx.prisma.score.create({
         data: {
-          traceId: trace.id,
+          projectId: input.projectId,
+          traceId: input.traceId,
           observationId: input.observationId,
           value: input.value,
           name: input.name,
           comment: input.comment,
-          projectId: trace.projectId,
           source: "REVIEW",
         },
       });
       await auditLog({
-        projectId: trace.projectId,
+        projectId: input.projectId,
         userId: ctx.session.user.id,
         userProjectRole: ctx.session.user.projects.find(
           (p) => p.id === trace.projectId,
@@ -190,114 +178,91 @@ export const scoresRouter = createTRPCRouter({
       });
       return score;
     }),
-  updateReviewScore: protectedProcedure
+  updateReviewScore: protectedProjectProcedure
     .input(
       z.object({
+        projectId: z.string(),
         id: z.string(),
         value: z.number(),
         comment: z.string().optional(),
       }),
     )
     .mutation(async ({ input, ctx }) => {
-      const score = await ctx.prisma.score.findFirstOrThrow({
+      throwIfNoAccess({
+        session: ctx.session,
+        projectId: input.projectId,
+        scope: "scores:CUD",
+      });
+      const score = await ctx.prisma.score.findFirst({
         where: {
           id: input.id,
+          projectId: input.projectId,
           source: "REVIEW",
-          trace: {
-            project: {
-              projectMembers: {
-                some: {
-                  userId: ctx.session.user.id,
-                },
-              },
-            },
-          },
         },
-        include: {
-          trace: {
-            select: {
-              projectId: true,
-            },
-          },
-        },
-      });
-      throwIfNoAccess({
-        session: ctx.session,
-        projectId: score.trace.projectId,
-        scope: "scores:CUD",
       });
+      if (!score) {
+        throw new Error("No review score with this id in this project.");
+      }
 
-      // exclude trace object from audit log
-      // eslint-disable-next-line @typescript-eslint/no-unused-vars
-      const { trace, ...pureScore } = score;
       await auditLog({
-        projectId: trace.projectId,
+        projectId: input.projectId,
         userId: ctx.session.user.id,
         userProjectRole: ctx.session.user.projects.find(
-          (p) => p.id === trace.projectId,
+          (p) => p.id === input.projectId,
         )?.role as ProjectRole, // throwIfNoAccess ensures this is defined
         resourceType: "score",
         resourceId: score.id,
         action: "update",
-        after: pureScore,
+        after: score,
       });
 
       return ctx.prisma.score.update({
         where: {
           id: score.id,
+          projectId: input.projectId,
         },
         data: {
           value: input.value,
           comment: input.comment,
         },
       });
     }),
-  deleteReviewScore: protectedProcedure
-    .input(z.string())
+  deleteReviewScore: protectedProjectProcedure
+    .input(z.object({ projectId: z.string(), id: z.string() }))
     .mutation(async ({ input, ctx }) => {
-      const score = await ctx.prisma.score.findFirstOrThrow({
-        where: {
-          id: input,
-          source: "REVIEW",
-          trace: {
-            project: {
-              projectMembers: {
-                some: {
-                  userId: ctx.session.user.id,
-                },
-              },
-            },
-          },
-        },
-        include: {
-          trace: {
-            select: {
-              projectId: true,
-            },
-          },
-        },
-      });
       throwIfNoAccess({
         session: ctx.session,
-        projectId: score.trace.projectId,
+        projectId: input.projectId,
         scope: "scores:CUD",
       });
-      const { trace, ...pureScore } = score;
+
+      const score = await ctx.prisma.score.findFirst({
+        where: {
+          id: input.id,
+          source: "REVIEW",
+          projectId: input.projectId,
+        },
+      });
+      if (!score) {
+        throw new Error("No review score with this id in this project.");
+      }
+
       await auditLog({
-        projectId: trace.projectId,
+        projectId: input.projectId,
         userId: ctx.session.user.id,
         userProjectRole: ctx.session.user.projects.find(
-          (p) => p.id === trace.projectId,
+          (p) => p.id === input.projectId,
         )?.role as ProjectRole, // throwIfNoAccess ensures this is defined
         resourceType: "score",
         resourceId: score.id,
         action: "delete",
-        before: pureScore,
+        before: score,
       });
 
       return ctx.prisma.score.delete({
         where: {
           id: score.id,
+          projectId: input.projectId,
         },
       });
     }),
@@ -316,7 +281,7 @@ const generateScoresQuery = (
    ${select}
   FROM scores s
   JOIN traces t ON t.id = s.trace_id LEFT JOIN job_executions je ON je.job_output_score_id = s.id AND je.project_id = ${projectId}
-  WHERE t.project_id = ${projectId}
+  WHERE s.project_id = ${projectId} AND t.project_id = ${projectId}
   ${filterCondition}
   ${orderCondition}
   LIMIT ${limit}