fix: address VORP code review issues (#283)

* fix: address VORP code review issues

- Switch admin EV form to batchUpsertParticipantEVs to avoid N×N
  concurrent UPDATE storm (was calling upsertParticipantEV per
  participant, each triggering a full syncVorpForSeason)
- deleteParticipantEV now resets vorpValue to "0" and calls
  syncVorpForSeason so remaining participants' ranks stay correct
- syncVorpForSeason issues a single bulk CASE UPDATE instead of
  N individual UPDATE statements
- Add doc warning on recalculateEV that callers must sync VORP manually
- Extract REPLACEMENT_LEVEL_START/END_IDX constants; clarify comment
  that 12-14 is a fixed product decision, not derived from league size
- Include vorpValue in draft room participant select projection
- Update drizzle-orm mock to support sql.join; update test assertions
  to reflect single bulk-update call

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: resolve lint errors (toSorted, unused var)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

app/models/__tests__/participant-expected-value.test.ts

@@ -14,7 +14,7 @@ import type { ProbabilityDistribution, ScoringRules } from "~/services/ev-calcul
 // Mock database context
 const mockUpdate = vi.fn();
 const mockSet = vi.fn();
-const mockWhere = vi.fn();
+const _mockWhere = vi.fn();
 const mockDb = {
   update: mockUpdate,
   select: vi.fn(),
@@ -32,11 +32,15 @@ vi.mock("~/database/schema", () => ({
   },
 }));
 
+const mockSqlFn = Object.assign(vi.fn(() => ({})), {
+  join: vi.fn(() => ({})),
+});
+
 vi.mock("drizzle-orm", () => ({
   eq: vi.fn((field, value) => ({ field, value })),
   and: vi.fn((...args) => ({ and: args })),
   count: vi.fn(() => ({ count: true })),
-  sql: vi.fn(),
+  sql: mockSqlFn,
 }));
 
 describe("participant-expected-value model", () => {
@@ -230,16 +234,14 @@ describe("participant-expected-value model", () => {
 
       await syncVorpForSeason("season-1");
 
-      // Should have called db.update 3 times (once per participant)
+      // Bulk update: db.update is called once for all participants
-      expect(mockUpdate).toHaveBeenCalledTimes(3);
+      expect(mockUpdate).toHaveBeenCalledTimes(1);
 
-      // Verify the set calls include vorpValue
+      // set() is called once with a CASE expression for vorpValue
-      const setCalls = mockSet.mock.calls;
+      expect(mockSet).toHaveBeenCalledTimes(1);
-      const vorpValues = setCalls.map((call) => call[0].vorpValue);
+      const setArg = mockSet.mock.calls[0][0];
-
+      expect(setArg).toHaveProperty("vorpValue");
-      expect(vorpValues).toContain("60.0000");
+      expect(setArg).toHaveProperty("updatedAt");
-      expect(vorpValues).toContain("30.0000");
-      expect(vorpValues).toContain("0.0000");
     });
   });
 });

app/models/participant-expected-value.ts

@@ -63,7 +63,7 @@ export async function syncVorpForSeason(sportsSeasonId: string): Promise<void> {
   const allEvs = await getAllParticipantEVsForSeason(sportsSeasonId);
   if (allEvs.length === 0) return;
 
-  const sorted = [...allEvs].sort(
+  const sorted = [...allEvs].toSorted(
     (a, b) => parseFloat(b.expectedValue) - parseFloat(a.expectedValue)
   );
 
@@ -72,15 +72,22 @@ export async function syncVorpForSeason(sportsSeasonId: string): Promise<void> {
 
   const now = new Date();
 
-  await Promise.all(
+  // Build a single CASE expression to update all participants in one query
+  // instead of N individual UPDATE statements.
+  const vorpCaseExpr = sql`CASE ${sql.join(
     sorted.map((ev) => {
       const vorp = calculateVORP(parseFloat(ev.expectedValue), replacementLevel);
-      return db
+      return sql`WHEN ${participants.id} = ${ev.participantId} THEN ${vorp.toFixed(4)}::numeric`;
-        .update(participants)
+    }),
-        .set({ vorpValue: vorp.toFixed(4), updatedAt: now })
+    sql` `
-        .where(eq(participants.id, ev.participantId));
+  )} END`;
-    })
+
-  );
+  const ids = sorted.map((ev) => ev.participantId);
+
+  await db
+    .update(participants)
+    .set({ vorpValue: vorpCaseExpr, updatedAt: now })
+    .where(sql`${participants.id} = ANY(${sql`ARRAY[${sql.join(ids.map((id) => sql`${id}`), sql`, `)}]::uuid[]`})`);
 }
 
 /**
@@ -258,11 +265,14 @@ export async function deleteParticipantEV(
       )
     );
 
-  // Reset participant EV to 0
+  // Reset this participant's EV and VORP to 0 (no longer ranked)
   await db
     .update(participants)
-    .set({ expectedValue: "0", updatedAt: new Date() })
+    .set({ expectedValue: "0", vorpValue: "0", updatedAt: new Date() })
     .where(eq(participants.id, participantId));
+
+  // Recalculate VORP for remaining participants — replacement level may have shifted
+  await syncVorpForSeason(sportsSeasonId);
 }
 
 /**
@@ -465,8 +475,12 @@ export function toProbabilityDistribution(ev: ParticipantEV): ProbabilityDistrib
 }
 
 /**
- * Recalculate EV for a participant with new scoring rules
+ * Recalculate EV for a participant with new scoring rules.
- * Keeps probabilities the same, only updates EV based on new scoring
+ * Keeps probabilities the same, only updates EV based on new scoring.
+ *
+ * NOTE: This function does NOT sync VORP. If calling this directly (outside
+ * of recalculateAllEVsForSeason), call syncVorpForSeason(sportsSeasonId)
+ * afterwards to keep draft order correct.
  */
 export async function recalculateEV(
   participantId: string,

app/routes/admin.sports-seasons.$id.expected-values.server.ts

@@ -3,7 +3,7 @@ import { logger } from "~/lib/logger";
 import { findSportsSeasonById } from "~/models/sports-season";
 import { findParticipantsBySportsSeasonId } from "~/models/participant";
 import {
-  upsertParticipantEV,
+  batchUpsertParticipantEVs,
   getAllParticipantEVsForSeason
 } from "~/models/participant-expected-value";
 
@@ -45,35 +45,24 @@ export async function action({ request, params }: Route.ActionArgs) {
   const participantIds = participants.map((p: { id: string }) => p.id);
 
   try {
-    const results = await Promise.all(
+    const inputs = participantIds.map((participantId) => ({
-      participantIds.map(async (participantId) => {
+      participantId,
-        const probFirst = parseFloat(formData.get(`probFirst_${participantId}`) as string || "0") / 100;
+      sportsSeasonId: params.id,
-        const probSecond = parseFloat(formData.get(`probSecond_${participantId}`) as string || "0") / 100;
+      probabilities: {
-        const probThird = parseFloat(formData.get(`probThird_${participantId}`) as string || "0") / 100;
+        probFirst: parseFloat(formData.get(`probFirst_${participantId}`) as string || "0") / 100,
-        const probFourth = parseFloat(formData.get(`probFourth_${participantId}`) as string || "0") / 100;
+        probSecond: parseFloat(formData.get(`probSecond_${participantId}`) as string || "0") / 100,
-        const probFifth = parseFloat(formData.get(`probFifth_${participantId}`) as string || "0") / 100;
+        probThird: parseFloat(formData.get(`probThird_${participantId}`) as string || "0") / 100,
-        const probSixth = parseFloat(formData.get(`probSixth_${participantId}`) as string || "0") / 100;
+        probFourth: parseFloat(formData.get(`probFourth_${participantId}`) as string || "0") / 100,
-        const probSeventh = parseFloat(formData.get(`probSeventh_${participantId}`) as string || "0") / 100;
+        probFifth: parseFloat(formData.get(`probFifth_${participantId}`) as string || "0") / 100,
-        const probEighth = parseFloat(formData.get(`probEighth_${participantId}`) as string || "0") / 100;
+        probSixth: parseFloat(formData.get(`probSixth_${participantId}`) as string || "0") / 100,
+        probSeventh: parseFloat(formData.get(`probSeventh_${participantId}`) as string || "0") / 100,
+        probEighth: parseFloat(formData.get(`probEighth_${participantId}`) as string || "0") / 100,
+      },
+      scoringRules,
+      source: "manual" as const,
+    }));
 
-        return upsertParticipantEV({
+    const results = await batchUpsertParticipantEVs(inputs);
-          participantId,
-          sportsSeasonId: params.id,
-          probabilities: {
-            probFirst,
-            probSecond,
-            probThird,
-            probFourth,
-            probFifth,
-            probSixth,
-            probSeventh,
-            probEighth,
-          },
-          scoringRules,
-          source: "manual",
-        });
-      })
-    );
 
     const totalEV = results.reduce((sum, r) => sum + parseFloat(r.expectedValue), 0);
 

app/routes/leagues/$leagueId.draft.$seasonId.tsx

@@ -161,6 +161,7 @@ export async function loader(args: Route.LoaderArgs) {
         .select({
           id: schema.participants.id,
           name: schema.participants.name,
+          vorpValue: schema.participants.vorpValue,
           sport: schema.sports,
         })
         .from(schema.participants)

app/services/ev-calculator.ts

@@ -169,12 +169,19 @@ export function calculateProjectedTotal(
   };
 }
 
+// Replacement level is defined as the average EV of players ranked 12th–14th
+// (1-indexed). This range is a fixed product decision that applies regardless
+// of actual league size — it represents the typical last pick in a standard draft.
+const REPLACEMENT_LEVEL_START_IDX = 11; // 0-indexed (1-indexed position 12)
+const REPLACEMENT_LEVEL_END_IDX = 13;   // 0-indexed (1-indexed position 14)
+
 /**
  * Calculate the replacement level EV for a sport's season.
  *
  * Replacement level = average EV of players ranked 12th–14th (1-indexed).
- * These are the last picks in a standard 12-team draft, representing the
+ * This is a fixed constant by product decision — it does not vary by league
- * baseline any manager could pick up off the waiver wire.
+ * size. It represents the baseline value any manager could expect from the
+ * last drafter-worthy player in a sport.
  *
  * @param sortedEvs - All participant EVs for a sport season, sorted descending
  * @returns Replacement level EV
@@ -182,9 +189,8 @@ export function calculateProjectedTotal(
 export function calculateReplacementLevel(sortedEvs: number[]): number {
   if (sortedEvs.length === 0) return 0;
 
-  // Target: average EVs at 1-indexed positions 12-14 (0-indexed: 11-13)
+  const startIdx = Math.min(REPLACEMENT_LEVEL_START_IDX, sortedEvs.length - 1);
-  const startIdx = Math.min(11, sortedEvs.length - 1);
+  const endIdx = Math.min(REPLACEMENT_LEVEL_END_IDX, sortedEvs.length - 1);
-  const endIdx = Math.min(13, sortedEvs.length - 1);
   const slice = sortedEvs.slice(startIdx, endIdx + 1);
   const avg = slice.reduce((sum, ev) => sum + ev, 0) / slice.length;
   return avg;