From 95f0e0612fc68d5483201c4d95c438d276216060 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Tue, 30 Jun 2026 16:30:23 +0000
Subject: [PATCH] Fix metadata clobbering and misleading auto-run message

Address two issues from code review:

- batchUpsertParticipantSimulatorInputs blindly COALESCE'd the metadata
  column, preserving a stale sourceEloMethod/ratingMethod flag. A freshly
  entered direct Elo/rating (Elo-ratings page, or the bulk CSV importer) was
  then misclassified as generated and filtered out by
  getParticipantSimulatorInputs, so the entered value was silently ignored.
  Metadata now uses explicit caller metadata when given, otherwise preserves
  existing flags but drops the method flag for any column receiving a fresh
  direct value.
- The save-inputs auto-run catch reported success with "Simulation not run
  yet" even when the simulation started and failed mid-run (leaving the season
  in status 'failed'). It now re-checks the season status and reports a real
  run failure distinctly from a not-ready/never-started run.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHnoQ7myY3PzNdTnd7iGNE
---
 .../__tests__/futures-odds-simulator.test.ts      |  9 +++++++++
 app/models/simulator.ts                           | 15 ++++++++++++++-
 app/routes/admin.sports-seasons.$id.simulator.tsx | 13 ++++++++++---
 3 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/app/models/__tests__/futures-odds-simulator.test.ts b/app/models/__tests__/futures-odds-simulator.test.ts
index a2dfd63..c2c8342 100644
--- a/app/models/__tests__/futures-odds-simulator.test.ts
+++ b/app/models/__tests__/futures-odds-simulator.test.ts
@@ -69,6 +69,15 @@ describe("batchUpsertParticipantSimulatorInputs", () => {
     expect(sourceEloSql).toContain("coalesce");
     const ratingSql = sqlToText(simulatorSet.rating).toLowerCase();
     expect(ratingSql).toContain("coalesce");
+
+    // Metadata must drop the per-column method flag whenever that column gets a
+    // fresh direct value, so a stale "generated" flag can't hide a newly entered
+    // Elo/rating. Blindly preserving metadata (e.g. COALESCE) would be the bug.
+    const metadataSql = sqlToText(simulatorSet.metadata).toLowerCase();
+    expect(metadataSql).toContain("sourceelomethod");
+    expect(metadataSql).toContain("ratingmethod");
+    expect(metadataSql).toContain("excluded.source_elo");
+    expect(metadataSql).toContain("excluded.rating");
   });
 
   it("is a no-op when given no inputs", async () => {
diff --git a/app/models/simulator.ts b/app/models/simulator.ts
index 8e70274..b41469f 100644
--- a/app/models/simulator.ts
+++ b/app/models/simulator.ts
@@ -353,7 +353,20 @@ export async function batchUpsertParticipantSimulatorInputs(
           projectedTablePoints: sql`COALESCE(excluded.projected_table_points, ${schema.seasonParticipantSimulatorInputs.projectedTablePoints})`,
           seed: sql`COALESCE(excluded.seed, ${schema.seasonParticipantSimulatorInputs.seed})`,
           region: sql`COALESCE(excluded.region, ${schema.seasonParticipantSimulatorInputs.region})`,
-          metadata: sql`COALESCE(excluded.metadata, ${schema.seasonParticipantSimulatorInputs.metadata})`,
+          // Metadata carries the method flags (sourceEloMethod/ratingMethod) that
+          // tell readers whether the stored Elo/rating is generated vs. a trusted
+          // direct value. When a caller supplies explicit metadata, use it as-is
+          // (prepareSimulatorInputsForRun and the projection importer set the
+          // correct flags). Otherwise preserve existing metadata, but drop the
+          // method flag for any column receiving a fresh direct value — otherwise a
+          // stale "generated" flag would cause that newly-entered Elo/rating to be
+          // filtered out as derived (see getParticipantSimulatorInputs).
+          metadata: sql`CASE
+            WHEN excluded.metadata IS NOT NULL THEN excluded.metadata
+            ELSE COALESCE(${schema.seasonParticipantSimulatorInputs.metadata}, '{}'::jsonb)
+              - (CASE WHEN excluded.source_elo IS NOT NULL THEN 'sourceEloMethod' ELSE '' END)
+              - (CASE WHEN excluded.rating IS NOT NULL THEN 'ratingMethod' ELSE '' END)
+          END`,
           updatedAt: sql`excluded.updated_at`,
         },
       });
diff --git a/app/routes/admin.sports-seasons.$id.simulator.tsx b/app/routes/admin.sports-seasons.$id.simulator.tsx
index ad08195..578315c 100644
--- a/app/routes/admin.sports-seasons.$id.simulator.tsx
+++ b/app/routes/admin.sports-seasons.$id.simulator.tsx
@@ -280,14 +280,21 @@ export async function action({ request, params }: Route.ActionArgs): Promise<Act
       const saved = `Saved ${parsed.inputs.length} simulator input row(s).${suffix}`;
 
       // Auto-run the simulation so saved inputs immediately drive the standings.
-      // The save itself already succeeded, so a not-ready run (e.g. participants
-      // missing required inputs) is reported as success with the readiness gap —
-      // never as a failed save.
+      // The save itself already succeeded, so a run that never started (e.g.
+      // participants missing required inputs) is reported as success with the
+      // readiness gap — never as a failed save.
       try {
         await runSportsSeasonSimulation(sportsSeasonId);
         return redirect(`/admin/sports-seasons/${sportsSeasonId}/expected-values`);
       } catch (runError) {
         const reason = runError instanceof Error ? runError.message : "could not run.";
+        // Distinguish "saved but never ran" (readiness/already-running) from
+        // "started running and failed mid-run": the runner only flips the season
+        // to status 'failed' once the simulation itself throws.
+        const season = await findSportsSeasonById(sportsSeasonId);
+        if (season?.simulationStatus === "failed") {
+          return { success: false, message: `${saved} The simulation failed: ${reason}` };
+        }
         return { success: true, message: `${saved} Simulation not run yet: ${reason}` };
       }
     } catch (error) {