Fix metadata clobbering and misleading auto-run message
Some checks failed
🚀 Deploy / 🧪 Test (pull_request) Successful in 2m58s
🚀 Deploy / ʦ🔍 Typecheck & Lint (pull_request) Failing after 47s
🚀 Deploy / 🐳 Build (pull_request) Has been skipped
🚀 Deploy / 🚀 Deploy (pull_request) Has been skipped

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
This commit is contained in:
Claude 2026-06-30 16:30:23 +00:00
parent b50a2b8c09
commit 95f0e0612f
No known key found for this signature in database
3 changed files with 33 additions and 4 deletions

View file

@ -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 () => {

View file

@ -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`,
},
});

View file

@ -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) {